Re: RFR 8194260, Point-to-point interface should be excluded from java/net/ipv6tests/*
On 2018/4/18 14:07, vyom tewari wrote: Hi Felix, latest code looks good to me, personally i prefer to throw exception instead returning null, but i can see that old code was also returning null. Yes, that was initial version, but it will make the test failing on host without IPv6 configured. With null, those tests just quit. -Felix Hopping the code which invokes getFirstLocalIPv4Address, getFirstLocalIPv6Addres already taken care of null. Thanks, Vyom On Wednesday 18 April 2018 11:05 AM, Felix Yang wrote: Hi Chris and Wyom, fixed as commented. Updated webrev: http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.01/ Thanks, Felix On 2018/4/17 16:25, Chris Hegarty wrote: On 17 Apr 2018, at 04:34, Felix Yang wrote: ... http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.00/ Thanks for doing this Felix. Mainly looks good. Just a few comments. The old test runs on systems without IPv4 or IPv6 configured. So I think the Optional `get` should be replaced with `orElse`. Either that or update usage to check for the presence of a value in the Optional. The old test filters out the loopback address, in order to get “real” addresses. I think we should preserve this behaviour. Other filtering is done in the old tests too, but I don’t think it is really needed. --- diff --git a/test/jdk/java/net/ipv6tests/Tests.java b/test/jdk/java/net/ipv6tests/Tests.java --- a/test/jdk/java/net/ipv6tests/Tests.java +++ b/test/jdk/java/net/ipv6tests/Tests.java @@ -178,26 +178,28 @@ } public static Inet4Address getFirstLocalIPv4Address () { - return getNetworkConfig().ip4Addresses() - .findFirst() - .get(); + return networkConfig.ip4Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } public static Inet6Address getFirstLocalIPv6Address () { - return getNetworkConfig().ip6Addresses() - .findFirst() - .get(); + return networkConfig.ip6Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } + private static NetworkConfiguration networkConfig = getNetworkConfig(); + private static NetworkConfiguration getNetworkConfig() { - NetworkConfiguration cfg = null; try { - cfg = NetworkConfiguration.probe(); + return NetworkConfiguration.probe(); } catch (IOException e) { System.out.println("Failed to probe NetworkConfiguration"); throw new RuntimeException(e); } - return cfg; } -Chris.
Re: RFR 8194260, Point-to-point interface should be excluded from java/net/ipv6tests/*
Hi Felix, latest code looks good to me, personally i prefer to throw exception instead returning null, but i can see that old code was also returning null. Hopping the code which invokes getFirstLocalIPv4Address, getFirstLocalIPv6Addres already taken care of null. Thanks, Vyom On Wednesday 18 April 2018 11:05 AM, Felix Yang wrote: Hi Chris and Wyom, fixed as commented. Updated webrev: http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.01/ Thanks, Felix On 2018/4/17 16:25, Chris Hegarty wrote: On 17 Apr 2018, at 04:34, Felix Yang wrote: ... http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.00/ Thanks for doing this Felix. Mainly looks good. Just a few comments. The old test runs on systems without IPv4 or IPv6 configured. So I think the Optional `get` should be replaced with `orElse`. Either that or update usage to check for the presence of a value in the Optional. The old test filters out the loopback address, in order to get “real” addresses. I think we should preserve this behaviour. Other filtering is done in the old tests too, but I don’t think it is really needed. --- diff --git a/test/jdk/java/net/ipv6tests/Tests.java b/test/jdk/java/net/ipv6tests/Tests.java --- a/test/jdk/java/net/ipv6tests/Tests.java +++ b/test/jdk/java/net/ipv6tests/Tests.java @@ -178,26 +178,28 @@ } public static Inet4Address getFirstLocalIPv4Address () { - return getNetworkConfig().ip4Addresses() - .findFirst() - .get(); + return networkConfig.ip4Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } public static Inet6Address getFirstLocalIPv6Address () { - return getNetworkConfig().ip6Addresses() - .findFirst() - .get(); + return networkConfig.ip6Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } + private static NetworkConfiguration networkConfig = getNetworkConfig(); + private static NetworkConfiguration getNetworkConfig() { - NetworkConfiguration cfg = null; try { - cfg = NetworkConfiguration.probe(); + return NetworkConfiguration.probe(); } catch (IOException e) { System.out.println("Failed to probe NetworkConfiguration"); throw new RuntimeException(e); } - return cfg; } -Chris.
Re: RFR 8194260, Point-to-point interface should be excluded from java/net/ipv6tests/*
> On 18 Apr 2018, at 06:35, Felix Yang wrote: > > Hi Chris and Wyom, > > fixed as commented. Updated webrev: > > http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.01/ Looks good, -Chris. > Thanks, > Felix > On 2018/4/17 16:25, Chris Hegarty wrote: >>> On 17 Apr 2018, at 04:34, Felix Yang wrote: >>> ... >>> http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.00/ >> Thanks for doing this Felix. Mainly looks good. Just a few comments. >> >> The old test runs on systems without IPv4 or IPv6 configured. So >> I think the Optional `get` should be replaced with `orElse`. Either that >> or update usage to check for the presence of a value in the Optional. >> >> The old test filters out the loopback address, in order to get “real” >> addresses. I think we should preserve this behaviour. Other filtering >> is done in the old tests too, but I don’t think it is really needed. >> >> --- >> diff --git a/test/jdk/java/net/ipv6tests/Tests.java >> b/test/jdk/java/net/ipv6tests/Tests.java >> --- a/test/jdk/java/net/ipv6tests/Tests.java >> +++ b/test/jdk/java/net/ipv6tests/Tests.java >> @@ -178,26 +178,28 @@ >> } >>public static Inet4Address getFirstLocalIPv4Address () { >> -return getNetworkConfig().ip4Addresses() >> - .findFirst() >> - .get(); >> +return networkConfig.ip4Addresses() >> + .filter(a -> !a.isLoopbackAddress()) >> + .findFirst() >> + .orElse(null); >> } >>public static Inet6Address getFirstLocalIPv6Address () { >> -return getNetworkConfig().ip6Addresses() >> - .findFirst() >> - .get(); >> +return networkConfig.ip6Addresses() >> + .filter(a -> !a.isLoopbackAddress()) >> + .findFirst() >> + .orElse(null); >> } >> +private static NetworkConfiguration networkConfig = getNetworkConfig(); >> + >> private static NetworkConfiguration getNetworkConfig() { >> -NetworkConfiguration cfg = null; >> try { >> -cfg = NetworkConfiguration.probe(); >> +return NetworkConfiguration.probe(); >> } catch (IOException e) { >> System.out.println("Failed to probe NetworkConfiguration"); >> throw new RuntimeException(e); >> } >> -return cfg; >> } >> -Chris. >
Re: RFR 8194260, Point-to-point interface should be excluded from java/net/ipv6tests/*
Hi Chris and Wyom, fixed as commented. Updated webrev: http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.01/ Thanks, Felix On 2018/4/17 16:25, Chris Hegarty wrote: On 17 Apr 2018, at 04:34, Felix Yang wrote: ... http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.00/ Thanks for doing this Felix. Mainly looks good. Just a few comments. The old test runs on systems without IPv4 or IPv6 configured. So I think the Optional `get` should be replaced with `orElse`. Either that or update usage to check for the presence of a value in the Optional. The old test filters out the loopback address, in order to get “real” addresses. I think we should preserve this behaviour. Other filtering is done in the old tests too, but I don’t think it is really needed. --- diff --git a/test/jdk/java/net/ipv6tests/Tests.java b/test/jdk/java/net/ipv6tests/Tests.java --- a/test/jdk/java/net/ipv6tests/Tests.java +++ b/test/jdk/java/net/ipv6tests/Tests.java @@ -178,26 +178,28 @@ } public static Inet4Address getFirstLocalIPv4Address () { -return getNetworkConfig().ip4Addresses() - .findFirst() - .get(); +return networkConfig.ip4Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } public static Inet6Address getFirstLocalIPv6Address () { -return getNetworkConfig().ip6Addresses() - .findFirst() - .get(); +return networkConfig.ip6Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } +private static NetworkConfiguration networkConfig = getNetworkConfig(); + private static NetworkConfiguration getNetworkConfig() { -NetworkConfiguration cfg = null; try { -cfg = NetworkConfiguration.probe(); +return NetworkConfiguration.probe(); } catch (IOException e) { System.out.println("Failed to probe NetworkConfiguration"); throw new RuntimeException(e); } -return cfg; } -Chris.
Re: RFR [11] 8197564: HTTP Client implementation - JEP 321
On Tue, Apr 17, 2018 at 6:28 PM, Chris Hegarty wrote: > On 17 Apr 2018, at 17:52, Viktor Klang wrote: > > ... > > There is technical and non-technical effort required. It is non-trivial. > > That said, we’re making every effort possible to move this forward. > > > > Hi Chris, > > > > how can we help? > > Thank you for asking, but I think we have it under control. I hope > to have news on this in the next couple of weeks. > Ok! > > -Chris. -- Cheers, √
Re: RFR [11] 8197564: HTTP Client implementation - JEP 321
On 17 Apr 2018, at 17:52, Viktor Klang wrote: > ... > There is technical and non-technical effort required. It is non-trivial. > That said, we’re making every effort possible to move this forward. > > Hi Chris, > > how can we help? Thank you for asking, but I think we have it under control. I hope to have news on this in the next couple of weeks. -Chris.
Re: RFR [11] 8197564: HTTP Client implementation - JEP 321
On Tue, Apr 17, 2018 at 9:55 AM, Chris Hegarty wrote: > Simone, > > > On 16 Apr 2018, at 18:47, Simone Bordet wrote: > > > >> ... > > > > Out of curiosity, is this code implementing the ReactiveStreams TCK > > (in its Flow declination) ? > > The code should be compliant with the RS TCK. > > > I ask because I have recently implemented it for Jetty's Reactive > > HttpClient (https://github.com/jetty-project/jetty-reactive-httpclient) > > and found a few surprising failures. > > If you encounter failures can you please report those to us, since > we are not aware of such. > > > It will be great if this can be done because all ReactiveStreams > > implementations implement the ReactiveStreams TCK, and so there is an > > assumed baseline of how these libraries should work that is beneficial > > for interoperability. > > Of course, interoperability is of utmost importance. > > > The effort is not much: each Publisher/Processor/Subscriber > > implementation should just extend a ReactiveStream TCK class > > overriding a few methods, for example: > > https://github.com/jetty-project/jetty-reactive- > httpclient/blob/master/src/test/java/org/eclipse/jetty/ > reactive/client/internal/QueuedSinglePublisherTCKTest.java > > There is technical and non-technical effort required. It is non-trivial. > That said, we’re making every effort possible to move this forward. > Hi Chris, how can we help? > > -Chris. > > -- Cheers, √
Re: RFR [11] 8197564: HTTP Client implementation - JEP 321
Simone, > On 16 Apr 2018, at 18:47, Simone Bordet wrote: > >> ... > > Out of curiosity, is this code implementing the ReactiveStreams TCK > (in its Flow declination) ? The code should be compliant with the RS TCK. > I ask because I have recently implemented it for Jetty's Reactive > HttpClient (https://github.com/jetty-project/jetty-reactive-httpclient) > and found a few surprising failures. If you encounter failures can you please report those to us, since we are not aware of such. > It will be great if this can be done because all ReactiveStreams > implementations implement the ReactiveStreams TCK, and so there is an > assumed baseline of how these libraries should work that is beneficial > for interoperability. Of course, interoperability is of utmost importance. > The effort is not much: each Publisher/Processor/Subscriber > implementation should just extend a ReactiveStream TCK class > overriding a few methods, for example: > https://github.com/jetty-project/jetty-reactive-httpclient/blob/master/src/test/java/org/eclipse/jetty/reactive/client/internal/QueuedSinglePublisherTCKTest.java There is technical and non-technical effort required. It is non-trivial. That said, we’re making every effort possible to move this forward. -Chris.
Re: RFR 8194260, Point-to-point interface should be excluded from java/net/ipv6tests/*
> On 17 Apr 2018, at 04:34, Felix Yang wrote: > ... > http://cr.openjdk.java.net/~xiaofeya/8194260/webrev.00/ Thanks for doing this Felix. Mainly looks good. Just a few comments. The old test runs on systems without IPv4 or IPv6 configured. So I think the Optional `get` should be replaced with `orElse`. Either that or update usage to check for the presence of a value in the Optional. The old test filters out the loopback address, in order to get “real” addresses. I think we should preserve this behaviour. Other filtering is done in the old tests too, but I don’t think it is really needed. --- diff --git a/test/jdk/java/net/ipv6tests/Tests.java b/test/jdk/java/net/ipv6tests/Tests.java --- a/test/jdk/java/net/ipv6tests/Tests.java +++ b/test/jdk/java/net/ipv6tests/Tests.java @@ -178,26 +178,28 @@ } public static Inet4Address getFirstLocalIPv4Address () { -return getNetworkConfig().ip4Addresses() - .findFirst() - .get(); +return networkConfig.ip4Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } public static Inet6Address getFirstLocalIPv6Address () { -return getNetworkConfig().ip6Addresses() - .findFirst() - .get(); +return networkConfig.ip6Addresses() + .filter(a -> !a.isLoopbackAddress()) + .findFirst() + .orElse(null); } +private static NetworkConfiguration networkConfig = getNetworkConfig(); + private static NetworkConfiguration getNetworkConfig() { -NetworkConfiguration cfg = null; try { -cfg = NetworkConfiguration.probe(); +return NetworkConfiguration.probe(); } catch (IOException e) { System.out.println("Failed to probe NetworkConfiguration"); throw new RuntimeException(e); } -return cfg; } -Chris.