[14:21:45] <AlexLehm> temporalfox: I posted a pr yesterday where i noticed a bug in it immediately after I posted it. I have fixed that now, I will think about that a bit further (since i will be offline anyway for the afternoon)

[14:22:03] <AlexLehm> the weather is way too good to spend the afternoon programming

[14:22:04] <temporalfox> AlexLehm in which repo ?

[14:22:11] <AlexLehm> vertx-mail-clinet

[14:22:17] <temporalfox> ok

[14:22:22] <temporalfox> so you're coming to F2F ?

[14:22:57] <AlexLehm> yes, i have registered the two days as vacation time, everything is checked

[14:23:01] <temporalfox> great

[14:23:08] <temporalfox> it's really cool you come

[14:23:28] <AlexLehm> yes, i think so too

[14:23:45] <AlexLehm> (not that I think it is cool i am coming, its cool that we can meet in general)

[14:38:11] <temporalfox> AlexLehm I think we should have more documentation about proxy support for HttpClient

[14:38:24] <temporalfox> document better what is possible and what is not

[14:38:43] <temporalfox> given taht NetClient and HttpClient don't exactly work the same

[14:38:49] <temporalfox> specially with TLS

[19:46:11] <BadApe> I am writing some integration tests, the strange thing is i don't seem to be able to make multiple calls from an http client, i am guessing i need to do something more after calling end()

[21:10:02] <AlexLehm> temporalfox: i think the operations now all work with proxy

[21:10:15] <AlexLehm> there should be no difference between the clients

[21:10:40] <temporalfox> but I think we said with non https we don't support connect for http client

[21:10:40] <temporalfox> don't we ?

[21:11:14] <temporalfox> ah

[21:11:59] <AlexLehm> yes, but the http client does a “normal” connection to the proxy, which isn't done in netclient, since it doesn't http

[21:13:23] <AlexLehm> I am almost sure that we have convered all use-cases necessary

[21:17:10] <temporalfox> but the issue we had recently in the vertx google group

[21:17:26] <temporalfox> wasn't about someone that wanted to use http with connect ?

[21:17:42] <temporalfox> I always mix-up these …

[21:18:21] <temporalfox> for httpclient and connect what are the rules ? can you remind me these ?

[21:18:25] <AlexLehm> yes, we fixed that in the 3.3.3 version

[21:19:20] <AlexLehm> httpclient uses connect when calling a https server by requesting the host/port address like CONNECT www.google.com:443

[21:20:18] <temporalfox> but the proxy server is called in clear ?

[21:20:26] <temporalfox> or it does not matter ?

[21:20:38] <AlexLehm> for http servers, a request for GET / HTTP/1.0 for www.google.de is translated to GET http://www.google.de/ HTTP/1.0 to the proxy

[21:21:13] <AlexLehm> the CONNECT request to the proxy is sent in clear but the ssl connection is then established to the origin server with ssl so that this doesn't matter

[21:21:15] <aesteve> evening everyone

[21:21:23] <temporalfox> hi arnayd

[21:21:24] <temporalfox> arnaud

[21:21:34] <temporalfox> AlexLehm ok

[21:21:40] <AlexLehm> that is the common use, https connections to the proxy are not used

[21:21:50] <temporalfox> so how it works depends on the HttpClient ssl=true|false configuration

[21:21:58] <aesteve> I made two PRs, not sure if this is interesting for other users too, but that's a pattern I'm facing quite often

[21:22:00] <temporalfox> ssl=true + proxy=http ⇒ CONNECT

[21:22:06] <AlexLehm> yes

[21:22:15] <temporalfox> ssl=false + proxy=http ⇒ rewrite requestURI

[21:22:22] <AlexLehm> es

[21:22:23] <AlexLehm> yes

[21:22:40] <temporalfox> I think AlexLehm it's not clear in documentation

[21:22:42] <temporalfox> do we document that properly ?

[21:23:27] <AlexLehm> sure, i will take a look

[21:23:32] <temporalfox> thanks

[21:23:48] <temporalfox> @aesteve in the vertx core PR , I've seen a put with Map.Entry<String, T>

[21:23:57] <temporalfox> I believe you should remove T and use “?” instead

[21:24:12] <temporalfox> that would be correct way to do in java

[21:24:21] <temporalfox> I can comment in the PR

[21:24:46] <aesteve> I'll correct it

[21:24:58] <aesteve> I'm always abit confused when it comes to generics

[21:25:18] <temporalfox> aesteve everyone is!

[21:26:21] <aesteve> I recently created a thin layer of the JPA Criteria API it drove me crazy with generics…. public <T, R extends Collection<? super V>, V> …

[21:26:46] <temporalfox> I hate super/extends

[21:26:51] <AlexLehm> its mentioned in the section about http client. but we can probably improve that http://vertx.io/docs/vertx-core/java/#_using_a_proxy_for_http_https_connections

[21:26:54] <temporalfox> we don't support them in codegen

[21:26:58] <temporalfox> and I think it's a good thing

[21:27:45] <temporalfox> AlexLehm I think that we should detail each case in a subsections

[21:27:49] <temporalfox> all this mix-up to me

[21:27:54] <temporalfox> with an example

[21:28:09] <aesteve> mmh actually temporalfox I broke my own stuff

[21:28:16] <temporalfox> like

[21:28:24] <temporalfox> client.get(…)

[21:28:25] <aesteve> don't merge it, it doesn't compile anylonger

[21:28:28] <temporalfox> commented as

[21:29:00] <temporalfox> client.get(443, “www.google.com”, “/”, …);

[21:29:03] <temporalfox> commented as

[21:29:26] <temporalfox> “ make a connect request on the proxy to establish a tunnel to https://www.google.com:443” [21:29:33] <temporalfox> so it is clear [21:29:40] <temporalfox> etc… [21:29:45] <temporalfox> and we make 4 sections [21:29:50] <temporalfox> I mean one for each sock [21:30:10] <temporalfox> one with connect proxy and one with rewrite proxy [21:30:38] <temporalfox> repetition and consistency I think is a good thing in documentation [21:30:44] <temporalfox> in such case [21:31:30] <aesteve> ok fixed [21:31:35] <aesteve> hope this is working now [21:31:51] <aesteve> it's weird though : assertEquals(json.getInteger(1), (Integer)3); [21:32:02] <temporalfox> that's the wonder of java boxing [21:37:53] <aesteve> I was wondering about the name of the method [21:38:06] <aesteve> collector() in both classes can clash with static imports [21:38:18] <aesteve> + isn't java “standard” but vert.x standard [21:38:40] <temporalfox> what do you mean ? [21:38:53] <temporalfox> how about instead having a constructor with a stream in JsonObject and JsonArray ? [21:38:59] <aesteve> it's not getXXX but since it's a static method I think it shouldn't anyway [21:39:25] <temporalfox> maybe we can look into other existing libraries to see how they name collectors [21:39:46] <aesteve> yes maybe [21:40:06] <aesteve> and about the Stream constructor let me check, I guess JsonObject has one [21:41:59] <aesteve> no, just JsonObject.stream() and Json.asStream(iterable) [21:42:42] <aesteve> would it be better with a stream constructor or the collector ? wdyt ? [21:43:36] <aesteve> new JsonObject(list.stream().filter(…).map(…)); or list.stream().filter(…).map(…).collect(JsonObject.collector()); [21:44:04] <aesteve> or maybe both [21:44:23] <temporalfox> using collect(..) seems more like the usual pattern I think [21:44:53] <temporalfox> so we should rather go with that [21:46:20] <aesteve> oh [21:46:41] <aesteve> I named it entryCollector() [21:46:45] <aesteve> forgot about that [21:46:53] <aesteve> so no clash within static imports [21:48:24] <temporalfox> why ? [21:48:42] <temporalfox> ah it collect Map.Entry streams ? [21:50:08] <aesteve> yes [21:50:26] <aesteve> best would have been Tuple<String, ?> [21:50:38] <aesteve> actually, I was wondering [21:50:43] <temporalfox> not for java [21:51:02] <aesteve> instead of doing a collector, what about implementing Collector directly ? [21:51:14] <temporalfox> in ? [21:51:18] <aesteve> list.stream().filter(…).map(…).collect(new JsonObject()); [21:51:29] <aesteve> not sure it makes sense [21:51:46] <temporalfox> hard to tell [21:52:12] <temporalfox> there must be something wrong with that no ? [21:52:12] <aesteve> mmh it's absolutely better !! [21:52:17] <aesteve> check [21:52:27] <temporalfox> because it measn that the same object could be used several times [21:52:41] <temporalfox> i.e it couples the collector and the object itself [21:52:51] <aesteve> JsonObject myJson = new JsonObject().put(“something”, a); list.stream().filter().map().collect(myJson); [21:53:19] <aesteve> you can compose an existing jsonobject with the result of filtering a list [21:53:31] <temporalfox> we could also have [21:53:40] <temporalfox> collector(jsonObject) [21:53:42] <temporalfox> with an existing json object [21:53:54] <aesteve> yes why not [21:54:16] <temporalfox> I think just collector is fine [21:54:28] <temporalfox> but I'm not a fan of static import [21:54:35] <temporalfox> except for junit [21:54:38] <temporalfox> Assert.* [21:54:59] <aesteve> I'm using it a lot with HttpMethods.* and HttpHeaders.* [21:55:24] <aesteve> (GET, POST, PUT, DELETE) is so much better than (HttpMethod.GET, HttpMethod.POST, …) [21:55:43] <aesteve> so we're going with collector() [21:55:48] <aesteve> and collector(JsonObject) [21:56:06] <aesteve> ? [21:56:38] <aesteve> (actually I can't see the problem of having it implement the interface directly) but I'm far from being used to collectors [21:57:25] <temporalfox> aesteve can you have a look at the HttpClientBuilder thread on vertx-dev ? [21:57:35] <aesteve> sure [21:57:37] <temporalfox> there are corresponding branches in a few projects [21:57:51] <temporalfox> it's a usability feature for http client [21:57:58] <temporalfox> that will benefit to rxjava [21:58:40] <aesteve> never used RxJava yet [21:58:49] <aesteve> but it'll benefit for Java users too ? [21:59:06] <temporalfox> yes [22:01:09] <aesteve> https://github.com/eclipse/vert.x/blob/http-client-request-builder/src/test/java/io/vertx/test/core/HttpClientRequestBuilderTest.java#L61 [22:01:15] <aesteve> createPost, I guess [22:02:27] <temporalfox> yes [22:02:34] <temporalfox> but now it's really something not polished [22:03:08] <aesteve> I have to remember the traditional way of issuing a request [22:03:32] <aesteve> request.end(Buffer), right ? [22:03:45] <temporalfox> yes [22:03:51] <aesteve> the main change is that you can add the handler when you issue the request ? [22:03:57] <temporalfox> the idea here is to give a ReadStream<Buffer> [22:04:10] <temporalfox> and also to copy the builder [22:04:21] <temporalfox> so you can reuse builders several times [22:04:32] <temporalfox> so [22:04:33] <temporalfox> client.createGet(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, “/somepath”); [22:04:34] <temporalfox> post.putHeader(“Content-Length”, “” + expected.length()) [22:04:34] <aesteve> that's a very nice idea, it'll help for dealing with proxies [22:04:40] <temporalfox> can be reused multiples times [22:04:51] <temporalfox> with different ReadStream<Buffer> [22:04:57] <temporalfox> and also other idea yes is that [22:04:58] <temporalfox> you use [22:05:04] <aesteve> OK I get it [22:05:12] <temporalfox> request(Handler<AsyncResult<HttpClientResponse») [22:05:24] <temporalfox> so you don't need to set exceptionHandler on HttpClientRequest [22:05:32] <temporalfox> and only manage a single error in the async result [22:06:02] <temporalfox> so simplyfing [22:06:11] <aesteve> oh it's an asyncresult [22:06:14] <temporalfox> yes [22:06:20] <aesteve> that'll be a pain in some cases though [22:06:25] <aesteve> can't we have both ? [22:06:29] <temporalfox> both ? [22:06:38] <aesteve> oh no obviously [22:06:46] <aesteve> the type is Handler<…> [22:06:52] <temporalfox> if you don't use AsyncResult then you ignore failure [22:07:18] <aesteve> In fact I'm thinking about my use cases [22:07:32] <aesteve> I'm using clientRequests for 2 things : proxies and tests [22:08:03] <aesteve> for testing, idc about the exception, that'll crash the test, exactly what I need [22:08:36] <aesteve> for proxies actually, having the readstream and the re-usable stuff is a bless [22:09:16] <temporalfox> yes for proxies it make simple indeed [22:09:33] <temporalfox> inside it takes care of setting up the pump [22:09:42] <temporalfox> and stopping if if there is an error [22:10:19] <aesteve> if you want a veryconcrete use case, have a look at how : https://github.com/aesteve/vertx-proxy/blob/master/src/main/java/io/vertx/examples/proxy/handler/ProxyHandler.java#L52-L88 would be simplified [22:10:33] <aesteve> pure awesomeness [22:10:51] <aesteve> just create a builder in the constructor and use it for every incomingRequest [22:11:45] <temporalfox> indeed very similar the same code I have in the HttpClientBuilder [22:11:54] <aesteve> for sure [22:12:04] <temporalfox> now I'm working on HttpClientRequest.reset() [22:12:24] <temporalfox> to abort the request if something goes wrong in the ReadStream [22:12:33] <temporalfox> it works for Http2 as it's an HTTP2 feature [22:12:41] <temporalfox> I'm trying to get something similar for HTTP1 [22:12:47] <temporalfox> i.e close gracefully the connection [22:12:54] <temporalfox> specially if it's pipelined [22:13:49] <aesteve> sounds difficult [22:14:08] <aesteve> mmh the first commit I made was with the wrong git account [22:14:43] <temporalfox> with pipelined it shall try to wait until all previous responses have been received and then close the connection [22:14:54] <temporalfox> and not put it back in the pool [22:15:58] <aesteve> mmh and I can't squash because there's a merge commit in between [22:16:52] <temporalfox> redo a new PR with cherry-pick [22:19:51] <aesteve> yes I'm goind to do that [22:32:27] <aesteve> mmh actually, the third possible implementation for JsonObject [22:32:33] <aesteve> is to create a class… [22:32:44] <aesteve> the most obvious solution I didn't even think about [22:32:52] <aesteve> new JsonObjectCollector() [22:33:12] <aesteve> or new JsonObjectCollector(existingJsonObject) [22:43:24] <aesteve> temporalfox: http://stackoverflow.com/questions/22753755/how-to-add-elements-of-a-java8-stream-into-an-existing-list [22:43:33] <aesteve> I think it's a matter of parallelism [22:44:26] <aesteve> (the same question I was asking about JsonObject is accurate for Collection<?> too, why can't we collect() into an existing Collection) [22:47:09] <temporalfox> “The short answer is no, at least, not in general, you shouldn't use a Collector to modify an existing collection.” [22:47:11] <temporalfox> you mean that ? [22:47:48] <temporalfox> looks like it's quite over-engineered though [22:49:29] <temporalfox> interesting read though [22:49:54] <temporalfox> aesteve I would rather prefer the static collector() [22:50:03] <temporalfox> as it does not introduce a new visible type to the user [22:50:17] <aesteve> ok [22:50:33] <temporalfox> also one possible thing is to create a new JsonObject with a collected Map ? [22:50:45] <temporalfox> although I find that collect map is quite difficult [22:50:54] <temporalfox> collect(Collectors.toMap) [22:51:00] <temporalfox> because it's generic ? [22:51:03] <temporalfox> I mean not for entries [22:52:02] <temporalfox> for list cone could do [22:52:24] <temporalfox> new JsonArray(stream….collect(Collectors.toList())); [22:53:23] <temporalfox> my question is : is it really important to provide a collector over just having a constructor that takes a stream ? [22:56:17] <aesteve> i don't know :\ [22:57:35] <aesteve> quite often, I hate instantiating an object just for the sake of fitting a method signature [22:57:50] <temporalfox> ok [22:58:10] <aesteve> I'm telling myself : why do I have to create a map, when a JsonObject already knows how to deal with map entries [22:58:46] <aesteve> but for the collector(existingJson) I'm not really sure [22:59:07] <aesteve> because it's gonna both mutate the original object and return it, maybe it's confusing [23:07:54] <temporalfox> aesteve stackoverload say that collector(existing) is bad [23:08:22] <aesteve> ok [23:09:01] <temporalfox> because of collectors multithreaded

[23:09:48] <aesteve> yes that's what I thought

[23:09:51] <aesteve> I removed it

[23:12:10] <temporalfox> ok

[23:12:43] <aesteve> https://github.com/eclipse/vert.x/pull/1645

[23:12:49] <aesteve> that should be OK now

[23:13:06] <aesteve> correct username for the CLA, correct method names, correct tests

[23:14:11] <aesteve> as for the PR re. vertx-web I'm not sure this will be useful, but I've already implemented it in many projects and what decided me was this article : https://dzone.com/articles/new-in-spring-5-functional-web-framework?edition=216172&utm_source=Daily%20Digest&utm_medium=email&utm_campaign=dd%202016-09-25

[23:15:06] <aesteve> the “then” style is sometimes useful

[23:15:49] <aesteve> router.route(“/api/1”).handler(CorsHandler.create(“*”)).then(BodyHandler.create()).then(this::setContentType)…

[23:16:04] <aesteve> maybe “and” is a better keyword but since handlers are chained…

[23:18:39] <temporalfox> what does the then do ?

[23:19:00] <aesteve> just create another route with the same path, same http methods

[23:19:30] <aesteve> that's something quite surprising when you first deal with vertx-web

[23:19:56] <aesteve> you can't do : Router.route(…).handler(1stHandler).handler(2ndHandler).handler(3rdHandler)

[23:20:19] <aesteve> you have to repeat the router.route(…) part

[23:20:31] <aesteve> (everytime you attach a handler)

[23:21:00] <aesteve> So I'm giving this solution a try, but maybe that's not the good one

[23:21:34] <aesteve> let's see what Paulo says

[23:21:45] <temporalfox> yep

[23:24:56] <aesteve> you could also imagine writing something like : router.routes(“/api/v1/*”, POST, PUT).handler(BodyHandler.create()).onlyFor(POST, this::addContentType)

[23:25:10] <aesteve> when you think about it that's abit like your httpclientbuilder

[23:25:26] <aesteve> I've already written a RouterBuilder but Groovy-DSL oriented

[23:25:35] <aesteve> maybe that's the idea, a RouterBuidler