Skip to content

Conversation

@guyarb
Copy link

@guyarb guyarb commented Mar 6, 2023

No description provided.

@guyarb guyarb mentioned this pull request Mar 6, 2023
} catch (ParseException e) {
System.err.println("Error parsing command line options: " + e.getMessage());
HelpFormatter formatter = new HelpFormatter();
formatter.printHelp("myapp", options);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, "");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

https://github.com/gaul/s3proxy/blob/master/Dockerfile

SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
sslContextFactory.setKeyStorePath(keystore);
sslContextFactory.setKeyStorePassword("123456");
sslContextFactory.setKeyManagerPassword("123456");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to support the 0 port for automatic assignment and use something like -1 to mean do not use HTTP?

@gaul
Copy link
Owner

gaul commented Oct 20, 2023

@guyarb do we have a path forward on this PR?

1 similar comment
@gaul
Copy link
Owner

gaul commented Oct 1, 2024

@guyarb do we have a path forward on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants