Conversation
| } catch (ParseException e) { | ||
| System.err.println("Error parsing command line options: " + e.getMessage()); | ||
| HelpFormatter formatter = new HelpFormatter(); | ||
| formatter.printHelp("myapp", options); |
There was a problem hiding this comment.
Should "myapp" be "httpbin" or args[0] or something sensible?
| @Before | ||
| public void setUp() throws Exception { | ||
| httpBin = new HttpBin(httpBinEndpoint); | ||
| httpBin = new HttpBin("127.0.0.1", 8001, 0, ""); |
There was a problem hiding this comment.
What is the intent here? Previously the tests used a zero port which selects a free port so it does not conflict with other running services. Please revert.
|
|
||
| RUN mvn clean install | ||
|
|
||
| FROM eclipse-temurin:17-jdk-jammy |
There was a problem hiding this comment.
Let's add a Dockerfile in a separate PR. But I am concerned with using such an old JDK instead of something more recent and streamlined like S3Proxy does:
| SslContextFactory.Server sslContextFactory = new SslContextFactory.Server(); | ||
| sslContextFactory.setKeyStorePath(keystore); | ||
| sslContextFactory.setKeyStorePassword("123456"); | ||
| sslContextFactory.setKeyManagerPassword("123456"); |
There was a problem hiding this comment.
Is this the best way to configure the keystore? Shouldn't it come from some external source like S3Proxy does?
| public final class HttpBin { | ||
| private final Server server; | ||
| private final int mHTTPPort; | ||
| private final int mHTTPsPort; |
There was a problem hiding this comment.
Let's not use the m prefix since nothing else does for members.
| mHTTPPort = httpPort; | ||
| mHTTPsPort = httpsPort; | ||
| List<Connector> connectors = new ArrayList<Connector>(); | ||
| if (httpPort != 0) { |
There was a problem hiding this comment.
Does it make sense to support the 0 port for automatic assignment and use something like -1 to mean do not use HTTP?
|
@guyarb do we have a path forward on this PR? |
1 similar comment
|
@guyarb do we have a path forward on this PR? |
2 participants
No description provided.