You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jared Stewart <js...@pivotal.io> on 2017/06/21 22:48:00 UTC
Review Request 60347: GEODE-3097: GFSH works over HTTP with SSL
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60347/
-----------------------------------------------------------
Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
GEODE-3097: GFSH works over HTTP with SSL
Diffs
-----
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java 2da95a7c3f305aab4e615d4be7b14c19b9b31dbc
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 844e0322eecdc93c4d46c546f4df2d278c5f15cd
Diff: https://reviews.apache.org/r/60347/diff/1/
Testing
-------
Thanks,
Jared Stewart
Re: Review Request 60347: GEODE-3097: GFSH works over HTTP with SSL
Posted by Jared Stewart <js...@pivotal.io>.
> On June 29, 2017, 5:34 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
> > Line 63 (original), 66 (patched)
> > <https://reviews.apache.org/r/60347/diff/1/?file=1757905#file1757905line79>
> >
> > Looks like this test has some problems. The Rules are not marked with @Rule, so the test would be responsible for invoking before and after on each Rule. I've said this before: a Rule that's not used as a Rule is "astonishing" or "surprising" -- https://en.wikipedia.org/wiki/Principle_of_least_astonishment
The Rules are being rules in this case, they're just being called through a `RuleChain` since we need to ensure order. Do you have any suggestions for how to make that more apparent?
- Jared
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60347/#review179282
-----------------------------------------------------------
On June 21, 2017, 10:48 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60347/
> -----------------------------------------------------------
>
> (Updated June 21, 2017, 10:48 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3097: GFSH works over HTTP with SSL
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java 2da95a7c3f305aab4e615d4be7b14c19b9b31dbc
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 844e0322eecdc93c4d46c546f4df2d278c5f15cd
>
>
> Diff: https://reviews.apache.org/r/60347/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>
Re: Review Request 60347: GEODE-3097: GFSH works over HTTP with SSL
Posted by Kirk Lund <ki...@gmail.com>.
> On June 29, 2017, 5:34 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
> > Line 63 (original), 66 (patched)
> > <https://reviews.apache.org/r/60347/diff/1/?file=1757905#file1757905line79>
> >
> > Looks like this test has some problems. The Rules are not marked with @Rule, so the test would be responsible for invoking before and after on each Rule. I've said this before: a Rule that's not used as a Rule is "astonishing" or "surprising" -- https://en.wikipedia.org/wiki/Principle_of_least_astonishment
>
> Jared Stewart wrote:
> The Rules are being rules in this case, they're just being called through a `RuleChain` since we need to ensure order. Do you have any suggestions for how to make that more apparent?
Oops, I missed the RuleChain because I didn't open nearby lines. Sorry about that.
- Kirk
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60347/#review179282
-----------------------------------------------------------
On June 21, 2017, 10:48 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60347/
> -----------------------------------------------------------
>
> (Updated June 21, 2017, 10:48 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3097: GFSH works over HTTP with SSL
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java 2da95a7c3f305aab4e615d4be7b14c19b9b31dbc
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 844e0322eecdc93c4d46c546f4df2d278c5f15cd
>
>
> Diff: https://reviews.apache.org/r/60347/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>
Re: Review Request 60347: GEODE-3097: GFSH works over HTTP with SSL
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60347/#review179282
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
Line 63 (original), 66 (patched)
<https://reviews.apache.org/r/60347/#comment253929>
Looks like this test has some problems. The Rules are not marked with @Rule, so the test would be responsible for invoking before and after on each Rule. I've said this before: a Rule that's not used as a Rule is "astonishing" or "surprising" -- https://en.wikipedia.org/wiki/Principle_of_least_astonishment
- Kirk Lund
On June 21, 2017, 10:48 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60347/
> -----------------------------------------------------------
>
> (Updated June 21, 2017, 10:48 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3097: GFSH works over HTTP with SSL
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java 2da95a7c3f305aab4e615d4be7b14c19b9b31dbc
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 844e0322eecdc93c4d46c546f4df2d278c5f15cd
>
>
> Diff: https://reviews.apache.org/r/60347/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>
Re: Review Request 60347: GEODE-3097: GFSH works over HTTP with SSL
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60347/#review180091
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
Lines 125 (patched)
<https://reviews.apache.org/r/60347/#comment255108>
Remove catch block? Method already "throws Exception"
- Kirk Lund
On June 21, 2017, 10:48 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60347/
> -----------------------------------------------------------
>
> (Updated June 21, 2017, 10:48 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3097: GFSH works over HTTP with SSL
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java 2da95a7c3f305aab4e615d4be7b14c19b9b31dbc
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 844e0322eecdc93c4d46c546f4df2d278c5f15cd
>
>
> Diff: https://reviews.apache.org/r/60347/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>
Re: Review Request 60347: GEODE-3097: GFSH works over HTTP with SSL
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60347/#review178604
-----------------------------------------------------------
Ship it!
Ship It!
- Jinmei Liao
On June 21, 2017, 10:48 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60347/
> -----------------------------------------------------------
>
> (Updated June 21, 2017, 10:48 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3097: GFSH works over HTTP with SSL
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java 2da95a7c3f305aab4e615d4be7b14c19b9b31dbc
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 844e0322eecdc93c4d46c546f4df2d278c5f15cd
>
>
> Diff: https://reviews.apache.org/r/60347/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>