> On Oct. 3, 2016, 6:54 p.m., Udo Kohlmeyer wrote: > > geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java, > > line 200 > > <https://reviews.apache.org/r/52488/diff/1/?file=1518865#file1518865line200> > > > > I don't think you should pass null. The SSLConfigurationFactory will > > always provide you with a SSLConfig object. This object might be a > > ssl-enabled=false, but that is ok. I think this should should be replaced > > with: > > > > SSLConfigurationFactory.setDistributionConfig(new DistributionConfig); > > final Server jetty = JettyHelper.initJetty(null, 8090, > > SSLConfigurationFactory.getSSLConfigForComponent(SecurableCommunicationChannel.WEB));
DistributionConfig is an interface, and if I use DistributionConfigImpl(), I get compiling error as well. I understand your concern of "checking null", but by forcing callers to pass in a valid SSLConfig while there is no easy way for them to get a default-not-enabled one gives the caller unnecessary burden. We can either allow null here, or have SSLConfigurationFactory add a method to return a default-nothing-enabled config? - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52488/#review151207 ----------------------------------------------------------- On Oct. 3, 2016, 5:52 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52488/ > ----------------------------------------------------------- > > (Updated Oct. 3, 2016, 5:52 p.m.) > > > Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo > Kohlmeyer. > > > Repository: geode > > > Description > ------- > > GEODE-420: fix Pulse test when not using any SSLConfig > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java > 089dbacb056da84d6d6ceb36dad6f09b9ce73890 > > geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java > a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 > > geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java > 5024250f8ff3867e0f527d7ef5fbbd501e2d64ee > > Diff: https://reviews.apache.org/r/52488/diff/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Jinmei Liao > >
