-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove httpClient param from AbstractRest.init #5354
Conversation
XN137
commented
Oct 11, 2022
- the httpClient builder was effectively unused after Isolate http-level test from java client-level tests #5314
- also rename to initApi
- the httpClient builder was effectively unused - also rename to initApi
HttpClientBuilder.builder() | ||
// Abuse the authentication callback a bit to inject the noAcceptFilter into | ||
// the java client. | ||
.withAuthentication( | ||
(HttpAuthentication) client -> client.addRequestFilter(noAcceptFilter)) | ||
.withUri(httpClient.getBaseUri()) | ||
.withUri(nessieApiUri) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we only used the base uri from the passed in httpClient... so we should instead override initApi(URI)
.build(NessieApiV1.class); | ||
|
||
super.init(api, httpClient); | ||
super.initApi(api); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super.init
did not use httpClient
} | ||
|
||
protected void init(NessieApiV1 api, @Nullable HttpClient.Builder httpClient) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
httpClient
param is ignored here (it used to be stored into a field previously)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking to move the Nessie client construction code into a JUnit5 extension and make it automatic for v1 and v2 API tests (based on #5339), but for now this looks like a reasonable refactoring. Thanks for cleaning this up :)