Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client [v18]
On Tue, 10 May 2022 13:35:35 GMT, Daniel Fuchs wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Daniel's review suggestion - add a test to verify the behaviour of the
>> localAddress() default method implementation on HttpClient.Builder
>> - Daniel's review suggestion - remove reference to "Internet Protocol" in
>> javadoc
>
> test/jdk/java/net/httpclient/HttpClientBuilderTest.java line 268:
>
>> 266: /**
>> 267: * Tests the {@link
>> HttpClient,java.net.http.HttpClient.Builder#localAddress(InetAddress)} method
>> 268: * behaviour when that method is called on a builder returned by
>> {@link HttpClient#newBuilder()}
>
> /**
> * Tests the {@link
> HttpClient,java.net.http.HttpClient.Builder#localAddress(InetAddress)} method
> ```
> This `{@link` looks broken - the `HttpClient,` prefix probably need to be
> removed?
Indeed. That prefix wasn't intentional. I'm not sure how I ended up with that.
I just pushed an update to fix this. Thank you for flagging this.
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client [v18]
On Tue, 10 May 2022 12:37:47 GMT, Jaikiran Pai wrote: >> This change proposes to implement the enhancement noted in >> https://bugs.openjdk.java.net/browse/JDK-8209137. >> >> The change introduces a new API to allow applications to build a >> `java.net.http.HTTPClient` configured with a specific local address that >> will be used while creating `Socket`(s) for connections. > > Jaikiran Pai has updated the pull request incrementally with two additional > commits since the last revision: > > - Daniel's review suggestion - add a test to verify the behaviour of the > localAddress() default method implementation on HttpClient.Builder > - Daniel's review suggestion - remove reference to "Internet Protocol" in > javadoc I've triggered some internal CI runs against the current state of PR to see all works fine. Now that we have decided on the nature of this API, I'll go ahead and file a CSR for this. - PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client [v18]
On Tue, 10 May 2022 13:36:18 GMT, Daniel Fuchs wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Daniel's review suggestion - add a test to verify the behaviour of the
>> localAddress() default method implementation on HttpClient.Builder
>> - Daniel's review suggestion - remove reference to "Internet Protocol" in
>> javadoc
>
> test/jdk/java/net/httpclient/HttpClientBuilderTest.java line 283:
>
>> 281: * Tests that the default method implementation of
>> 282: * {@link
>> HttpClient,java.net.http.HttpClient.Builder#localAddress(InetAddress)} throws
>> 283: * an {@link UnsupportedOperationException}
>
> Same remark here
Otherwise things look good to me - you should update the CSR if it needs to be
updated.
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client [v18]
On Tue, 10 May 2022 12:37:47 GMT, Jaikiran Pai wrote:
>> This change proposes to implement the enhancement noted in
>> https://bugs.openjdk.java.net/browse/JDK-8209137.
>>
>> The change introduces a new API to allow applications to build a
>> `java.net.http.HTTPClient` configured with a specific local address that
>> will be used while creating `Socket`(s) for connections.
>
> Jaikiran Pai has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Daniel's review suggestion - add a test to verify the behaviour of the
> localAddress() default method implementation on HttpClient.Builder
> - Daniel's review suggestion - remove reference to "Internet Protocol" in
> javadoc
test/jdk/java/net/httpclient/HttpClientBuilderTest.java line 268:
> 266: /**
> 267: * Tests the {@link
> HttpClient,java.net.http.HttpClient.Builder#localAddress(InetAddress)} method
> 268: * behaviour when that method is called on a builder returned by
> {@link HttpClient#newBuilder()}
/**
* Tests the {@link
HttpClient,java.net.http.HttpClient.Builder#localAddress(InetAddress)} method
```
This `{@link` looks broken - the `HttpClient,` prefix probably need to be
removed?
test/jdk/java/net/httpclient/HttpClientBuilderTest.java line 283:
> 281: * Tests that the default method implementation of
> 282: * {@link
> HttpClient,java.net.http.HttpClient.Builder#localAddress(InetAddress)} throws
> 283: * an {@link UnsupportedOperationException}
Same remark here
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client [v18]
> This change proposes to implement the enhancement noted in > https://bugs.openjdk.java.net/browse/JDK-8209137. > > The change introduces a new API to allow applications to build a > `java.net.http.HTTPClient` configured with a specific local address that will > be used while creating `Socket`(s) for connections. Jaikiran Pai has updated the pull request incrementally with two additional commits since the last revision: - Daniel's review suggestion - add a test to verify the behaviour of the localAddress() default method implementation on HttpClient.Builder - Daniel's review suggestion - remove reference to "Internet Protocol" in javadoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/6690/files - new: https://git.openjdk.java.net/jdk/pull/6690/files/d4a19dea..3a6f2b6b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6690=17 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6690=16-17 Stats: 74 lines in 2 files changed: 70 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6690.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6690/head:pull/6690 PR: https://git.openjdk.java.net/jdk/pull/6690
