Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
Hi, I find the exposure of an implementation detail (usage of Apache HTTP client) of the Elasticsearch client a bit problematic. If they change this to another HTTP client, our SPI would break. Did you instead consider to just let users provide their custom instance of org.elasticsearch.client.RestClient? It's still leaking an implementation detail of Hibernate Search, but at least it's one indirection less. People wishing to have a custom RestClient would have to implement a few more bits themselves (the logic from DefaultElasticsearchClientFactory#customizeHttpClientConfig()), but I'd find that acceptable for the sake of a less detail-exposing SPI, plus it grants more flexibility in terms of configuring the RestClient. --Gunnar 2017-06-01 19:11 GMT+02:00 Sanne Grinovero : > Yoann has been working on allowing Hibernate Search users to use > Elasticsearch on AWS. > > Specifically on AWS the Elasticsearch security can be configured to > use application identities, which implies the requests need to be > signed. > > A good background read can be found here [1]. > > We planned to allow people to use this but were not planning to > include AWS specific libraries as dependencies - but since Yoann > implemented an actual AWS signer in the tests I suppose it would be > selfish to not ship it.. > > Please see the API proposal on github (with the PR): > - https://github.com/hibernate/hibernate-search/pull/1426 > > Thanks, > Sanne > > [1] - > https://aws.amazon.com/blogs/security/how-to-control-access-to-your-amazon-elasticsearch-service-domain/ > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
On Fri, 2 Jun 2017, 08:56 Gunnar Morling, wrote: > Hi, > > I find the exposure of an implementation detail (usage of Apache HTTP > client) of the Elasticsearch client a bit problematic. If they change > this to another HTTP client, our SPI would break. > Yes the very point of exposing that detail is the reason for this thread. Still, our SPI being guaranteed only for a minor, that gives a lot of flexibility? The Elasticsearch client exposing this itself, I don't expect them to switch implementation in a micro release to make some bugfix. If they change it in a major or even minor version, we're ok to not support that version until our next minor. > Did you instead consider to just let users provide their custom > instance of org.elasticsearch.client.RestClient? It's still leaking an > implementation detail of Hibernate Search, but at least it's one > indirection less. > > People wishing to have a custom RestClient would have to implement a > few more bits themselves (the logic from > DefaultElasticsearchClientFactory#customizeHttpClientConfig()), but > I'd find that acceptable for the sake of a less detail-exposing SPI, > plus it grants more flexibility in terms of configuring the > RestClient. > N.B. The client factory already is a Service so any advanced user already can override it. We want to make it easier for cloud users. Focusing on AWS now as we've had user requests for this - not least our own CI - but I'd expect other clouds to have similar features (today or tomorrow). I just don't expect other use cases to need this so we might provide them all eventually, but at this point my goal is to leave an appealing SPI for contributors to join on that. With that I mean: 1# this might evolve but we need something simple to use for people to not get stuck. 2# I expect integrator implementors to contribute them back 3# People won't have this low level dependency in their projects for long Having them re-implement the client wouldn't encourage this ;) Thanks! Sanne > --Gunnar > > > 2017-06-01 19:11 GMT+02:00 Sanne Grinovero : > > Yoann has been working on allowing Hibernate Search users to use > > Elasticsearch on AWS. > > > > Specifically on AWS the Elasticsearch security can be configured to > > use application identities, which implies the requests need to be > > signed. > > > > A good background read can be found here [1]. > > > > We planned to allow people to use this but were not planning to > > include AWS specific libraries as dependencies - but since Yoann > > implemented an actual AWS signer in the tests I suppose it would be > > selfish to not ship it.. > > > > Please see the API proposal on github (with the PR): > > - https://github.com/hibernate/hibernate-search/pull/1426 > > > > Thanks, > > Sanne > > > > [1] - > https://aws.amazon.com/blogs/security/how-to-control-access-to-your-amazon-elasticsearch-service-domain/ > > ___ > > hibernate-dev mailing list > > hibernate-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
2017-06-02 10:30 GMT+02:00 Sanne Grinovero : > > > On Fri, 2 Jun 2017, 08:56 Gunnar Morling, wrote: >> >> Hi, >> >> I find the exposure of an implementation detail (usage of Apache HTTP >> client) of the Elasticsearch client a bit problematic. If they change >> this to another HTTP client, our SPI would break. > > > Yes the very point of exposing that detail is the reason for this thread. > > Still, our SPI being guaranteed only for a minor, that gives a lot of > flexibility? > > The Elasticsearch client exposing this itself, I don't expect them to switch > implementation in a micro release to make some bugfix. If they change it in > a major or even minor version, we're ok to not support that version until > our next minor. It might be covered by the HSEARCH rules for evolvement, but personally I think it's just not a good practice to leak internals of a dependency in our SPI. > > >> >> Did you instead consider to just let users provide their custom >> instance of org.elasticsearch.client.RestClient? It's still leaking an >> implementation detail of Hibernate Search, but at least it's one >> indirection less. >> >> People wishing to have a custom RestClient would have to implement a >> few more bits themselves (the logic from >> DefaultElasticsearchClientFactory#customizeHttpClientConfig()), but >> I'd find that acceptable for the sake of a less detail-exposing SPI, >> plus it grants more flexibility in terms of configuring the >> RestClient. > > > > N.B. The client factory already is a Service so any advanced user already > can override it. The contract is defined under impl, though. Nothing users should consider to implement. Unless you mean to promote it to SPI proper? > > We want to make it easier for cloud users. Focusing on AWS now as we've had > user requests for this - not least our own CI - but I'd expect other clouds > to have similar features (today or tomorrow). I just don't expect other use > cases to need this so we might provide them all eventually, but at this > point my goal is to leave an appealing SPI for contributors to join on that. > > With that I mean: > 1# this might evolve but we need something simple to use for people to not > get stuck. > 2# I expect integrator implementors to contribute them back > 3# People won't have this low level dependency in their projects for long > > Having them re-implement the client wouldn't encourage this ;) You could provide an AWS-based implementation of the client factory either as an example or in a separate module. So people can easily make use of it, but usage of the HTTP client isn't exposed in core SPIs. Also that way you don't commit to a low-level SPI now but rather can gain experiences with the custom client factories people create and evolve from there. > > Thanks! > Sanne > >> >> --Gunnar >> >> >> 2017-06-01 19:11 GMT+02:00 Sanne Grinovero : >> > Yoann has been working on allowing Hibernate Search users to use >> > Elasticsearch on AWS. >> > >> > Specifically on AWS the Elasticsearch security can be configured to >> > use application identities, which implies the requests need to be >> > signed. >> > >> > A good background read can be found here [1]. >> > >> > We planned to allow people to use this but were not planning to >> > include AWS specific libraries as dependencies - but since Yoann >> > implemented an actual AWS signer in the tests I suppose it would be >> > selfish to not ship it.. >> > >> > Please see the API proposal on github (with the PR): >> > - https://github.com/hibernate/hibernate-search/pull/1426 >> > >> > Thanks, >> > Sanne >> > >> > [1] - >> > https://aws.amazon.com/blogs/security/how-to-control-access-to-your-amazon-elasticsearch-service-domain/ >> > ___ >> > hibernate-dev mailing list >> > hibernate-dev@lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
> > Did you instead consider to just let users provide their custom > instance of org.elasticsearch.client.RestClient? It's still leaking an > implementation detail of Hibernate Search, but at least it's one > indirection less. The only way to add AWS authentication to the RestClient is to use Apache HTTP Client classes, so this solution would still bind users to the implementation details. We'd just shove it under the carpet :) Also, I'd argue this would be an even bigger implementation leak: at least with the current solution with can switch to an alternative Elasticsearch client, as long as we still use Apache HTTP Client. If we expose RestClient, we're stuck with it *and* whatever underlying technologies it uses. On the other hand, we could ask them to re-implement org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient, which is our own façade over the Elasticsearch client. But I find it a bit much just to tweak some settings or to add a new authentication scheme... On top of that, this solution would not allow multiple third-party customizations to work well together (for instance AWS authentication provided by us or a third party + some performance tweaking by the user)... which is something the SPI we're planning in the PR could allow. Yoann Rodière Hibernate NoORM Team yo...@hibernate.org On 2 June 2017 at 10:30, Sanne Grinovero wrote: > On Fri, 2 Jun 2017, 08:56 Gunnar Morling, wrote: > > > Hi, > > > > I find the exposure of an implementation detail (usage of Apache HTTP > > client) of the Elasticsearch client a bit problematic. If they change > > this to another HTTP client, our SPI would break. > > > > Yes the very point of exposing that detail is the reason for this thread. > > Still, our SPI being guaranteed only for a minor, that gives a lot of > flexibility? > > The Elasticsearch client exposing this itself, I don't expect them to > switch implementation in a micro release to make some bugfix. If they > change it in a major or even minor version, we're ok to not support that > version until our next minor. > > > > > Did you instead consider to just let users provide their custom > > instance of org.elasticsearch.client.RestClient? It's still leaking an > > implementation detail of Hibernate Search, but at least it's one > > indirection less. > > > > People wishing to have a custom RestClient would have to implement a > > few more bits themselves (the logic from > > DefaultElasticsearchClientFactory#customizeHttpClientConfig()), but > > I'd find that acceptable for the sake of a less detail-exposing SPI, > > plus it grants more flexibility in terms of configuring the > > RestClient. > > > > > N.B. The client factory already is a Service so any advanced user already > can override it. > > We want to make it easier for cloud users. Focusing on AWS now as we've had > user requests for this - not least our own CI - but I'd expect other clouds > to have similar features (today or tomorrow). I just don't expect other use > cases to need this so we might provide them all eventually, but at this > point my goal is to leave an appealing SPI for contributors to join on > that. > > With that I mean: > 1# this might evolve but we need something simple to use for people to not > get stuck. > 2# I expect integrator implementors to contribute them back > 3# People won't have this low level dependency in their projects for long > > Having them re-implement the client wouldn't encourage this ;) > > Thanks! > Sanne > > > > --Gunnar > > > > > > 2017-06-01 19:11 GMT+02:00 Sanne Grinovero : > > > Yoann has been working on allowing Hibernate Search users to use > > > Elasticsearch on AWS. > > > > > > Specifically on AWS the Elasticsearch security can be configured to > > > use application identities, which implies the requests need to be > > > signed. > > > > > > A good background read can be found here [1]. > > > > > > We planned to allow people to use this but were not planning to > > > include AWS specific libraries as dependencies - but since Yoann > > > implemented an actual AWS signer in the tests I suppose it would be > > > selfish to not ship it.. > > > > > > Please see the API proposal on github (with the PR): > > > - https://github.com/hibernate/hibernate-search/pull/1426 > > > > > > Thanks, > > > Sanne > > > > > > [1] - > > https://aws.amazon.com/blogs/security/how-to-control- > access-to-your-amazon-elasticsearch-service-domain/ > > > ___ > > > hibernate-dev mailing list > > > hibernate-dev@lists.jboss.org > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
2017-06-02 11:58 GMT+02:00 Yoann Rodiere : >> Did you instead consider to just let users provide their custom >> instance of org.elasticsearch.client.RestClient? It's still leaking an >> implementation detail of Hibernate Search, but at least it's one >> indirection less. > > > The only way to add AWS authentication to the RestClient is to use Apache > HTTP Client classes, so this solution would still bind users to the > implementation details. We'd just shove it under the carpet :) There's an important difference: one exposes Apache HTTP client in HSEARCH's SPI, whereas the other just requires usage of Apache HTTP client within one specific implementation. For users it doesn't change much, but the latter is cleaner from HSEARCH's perspective. > > Also, I'd argue this would be an even bigger implementation leak: at least > with the current solution with can switch to an alternative Elasticsearch > client, as long as we still use Apache HTTP Client. If we expose RestClient, > we're stuck with it and whatever underlying technologies it uses. > > On the other hand, we could ask them to re-implement > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient, which is > our own façade over the Elasticsearch client. But I find it a bit much just > to tweak some settings or to add a new authentication scheme... > On top of that, this solution would not allow multiple third-party > customizations to work well together (for instance AWS authentication > provided by us or a third party + some performance tweaking by the user)... > which is something the SPI we're planning in the PR could allow. I'm not quite following on that. If people are in control of the RestClient entirely, they can do whatever they want? > > Yoann Rodière > Hibernate NoORM Team > yo...@hibernate.org > > On 2 June 2017 at 10:30, Sanne Grinovero wrote: >> >> On Fri, 2 Jun 2017, 08:56 Gunnar Morling, wrote: >> >> > Hi, >> > >> > I find the exposure of an implementation detail (usage of Apache HTTP >> > client) of the Elasticsearch client a bit problematic. If they change >> > this to another HTTP client, our SPI would break. >> > >> >> Yes the very point of exposing that detail is the reason for this thread. >> >> Still, our SPI being guaranteed only for a minor, that gives a lot of >> flexibility? >> >> The Elasticsearch client exposing this itself, I don't expect them to >> switch implementation in a micro release to make some bugfix. If they >> change it in a major or even minor version, we're ok to not support that >> version until our next minor. >> >> >> >> > Did you instead consider to just let users provide their custom >> > instance of org.elasticsearch.client.RestClient? It's still leaking an >> > implementation detail of Hibernate Search, but at least it's one >> > indirection less. >> > >> > People wishing to have a custom RestClient would have to implement a >> > few more bits themselves (the logic from >> > DefaultElasticsearchClientFactory#customizeHttpClientConfig()), but >> > I'd find that acceptable for the sake of a less detail-exposing SPI, >> > plus it grants more flexibility in terms of configuring the >> > RestClient. >> > >> >> >> N.B. The client factory already is a Service so any advanced user already >> can override it. >> >> We want to make it easier for cloud users. Focusing on AWS now as we've >> had >> user requests for this - not least our own CI - but I'd expect other >> clouds >> to have similar features (today or tomorrow). I just don't expect other >> use >> cases to need this so we might provide them all eventually, but at this >> point my goal is to leave an appealing SPI for contributors to join on >> that. >> >> With that I mean: >> 1# this might evolve but we need something simple to use for people to not >> get stuck. >> 2# I expect integrator implementors to contribute them back >> 3# People won't have this low level dependency in their projects for long >> >> Having them re-implement the client wouldn't encourage this ;) >> >> Thanks! >> Sanne >> >> >> > --Gunnar >> > >> > >> > 2017-06-01 19:11 GMT+02:00 Sanne Grinovero : >> > > Yoann has been working on allowing Hibernate Search users to use >> > > Elasticsearch on AWS. >> > > >> > > Specifically on AWS the Elasticsearch security can be configured to >> > > use application identities, which implies the requests need to be >> > > signed. >> > > >> > > A good background read can be found here [1]. >> > > >> > > We planned to allow people to use this but were not planning to >> > > include AWS specific libraries as dependencies - but since Yoann >> > > implemented an actual AWS signer in the tests I suppose it would be >> > > selfish to not ship it.. >> > > >> > > Please see the API proposal on github (with the PR): >> > > - https://github.com/hibernate/hibernate-search/pull/1426 >> > > >> > > Thanks, >> > > Sanne >> > > >> > > [1] - >> > >> > https://aws.amazon.com/blogs/security/how-to-control-access-to-your-amazon-elasticsearch-service-
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
On 2 June 2017 at 10:58, Yoann Rodiere wrote: >> Did you instead consider to just let users provide their custom >> instance of org.elasticsearch.client.RestClient? It's still leaking an >> implementation detail of Hibernate Search, but at least it's one >> indirection less. > > > The only way to add AWS authentication to the RestClient is to use Apache > HTTP Client classes, so this solution would still bind users to the > implementation details. We'd just shove it under the carpet :) > > Also, I'd argue this would be an even bigger implementation leak: at least > with the current solution with can switch to an alternative Elasticsearch > client, as long as we still use Apache HTTP Client. If we expose RestClient, > we're stuck with it and whatever underlying technologies it uses. > > On the other hand, we could ask them to re-implement > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient, which is > our own façade over the Elasticsearch client. But I find it a bit much just > to tweak some settings or to add a new authentication scheme... > On top of that, this solution would not allow multiple third-party > customizations to work well together (for instance AWS authentication > provided by us or a third party + some performance tweaking by the user)... > which is something the SPI we're planning in the PR could allow. +1 I think you nailed it design wise. Gunnar, I have the impression we're simply not aligned on the definition of SPI. How to do custom request signing on a *specific platform* (AWS) with a *specific client* (this ES client using the Apache HTTP client) is going to need a specific adaptor; the good news is that this is going to be isolated so the main user application code - entities and business logic - won't be affected. That's SPI by the book and we're not giving guarantees of backwards compatibility beyond a minor. We know the Elasticsearch driver is using the Apache HTTP client and this won't change overnight; if they happen to change it, we'll change in a new minor release and people will have to adapt. Too bad but it's practical. Let me also point out a drawback of your alternative solution: the user's code for those we re-implement the driver would not break but only if they fail to upgrade the driver, which is possibly another pandora box. If you feel strongly about it I'm ok to promote the `DefaultElasticsearchClientFactory` to SPI to give some more options but I see no much value in that. Send a PR if you want it ;) Thanks, Sanne > > Yoann Rodière > Hibernate NoORM Team > yo...@hibernate.org > > On 2 June 2017 at 10:30, Sanne Grinovero wrote: >> >> On Fri, 2 Jun 2017, 08:56 Gunnar Morling, wrote: >> >> > Hi, >> > >> > I find the exposure of an implementation detail (usage of Apache HTTP >> > client) of the Elasticsearch client a bit problematic. If they change >> > this to another HTTP client, our SPI would break. >> > >> >> Yes the very point of exposing that detail is the reason for this thread. >> >> Still, our SPI being guaranteed only for a minor, that gives a lot of >> flexibility? >> >> The Elasticsearch client exposing this itself, I don't expect them to >> switch implementation in a micro release to make some bugfix. If they >> change it in a major or even minor version, we're ok to not support that >> version until our next minor. >> >> >> >> > Did you instead consider to just let users provide their custom >> > instance of org.elasticsearch.client.RestClient? It's still leaking an >> > implementation detail of Hibernate Search, but at least it's one >> > indirection less. >> > >> > People wishing to have a custom RestClient would have to implement a >> > few more bits themselves (the logic from >> > DefaultElasticsearchClientFactory#customizeHttpClientConfig()), but >> > I'd find that acceptable for the sake of a less detail-exposing SPI, >> > plus it grants more flexibility in terms of configuring the >> > RestClient. >> > >> >> >> N.B. The client factory already is a Service so any advanced user already >> can override it. >> >> We want to make it easier for cloud users. Focusing on AWS now as we've >> had >> user requests for this - not least our own CI - but I'd expect other >> clouds >> to have similar features (today or tomorrow). I just don't expect other >> use >> cases to need this so we might provide them all eventually, but at this >> point my goal is to leave an appealing SPI for contributors to join on >> that. >> >> With that I mean: >> 1# this might evolve but we need something simple to use for people to not >> get stuck. >> 2# I expect integrator implementors to contribute them back >> 3# People won't have this low level dependency in their projects for long >> >> Having them re-implement the client wouldn't encourage this ;) >> >> Thanks! >> Sanne >> >> >> > --Gunnar >> > >> > >> > 2017-06-01 19:11 GMT+02:00 Sanne Grinovero : >> > > Yoann has been working on allowing Hibernate Search users to use >> > > Elasticsearch on AW
[hibernate-dev] Announcing: Early-Access builds of JDK 9 for Alpine Linux/musl at jdk.java.net/9/
Hi Sanne, ** *Announcing: Early-Access builds of JDK 9 for Alpine Linux/musl at jdk.java.net/9/ [1] * * As of today there are pre-built Early-Access (EA) JDK binaries for Alpine Linux/musl at jdk.java.net/9/** o look for “Alpine Linux”. [1] * The Alpine Linux build is compatible with linux distributions that use the musl C library. *[2]* Feedback is very welcome via the portola-dev mailing list, remembering to subscribe to the mailing list first. *Proposed schedule change for JDK 9 [3]* JDK 9 Project continues to work toward the current goal of producing an initial Release Candidate build on 22 June. This proposal is to adjust the GA date in order to accommodate the additional time required to move through the JCP process. To be specific, we propose to move the GA date out by eight weeks, from 27 July to 21 September. Rgds,Rory [1] http://mail.openjdk.java.net/pipermail/portola-dev/2017-June/000191.html [2] http://www.musl-libc.org/ [3] http://mail.openjdk.java.net/pipermail/jdk9-dev/2017-May/005864.html -- Rgds,Rory O'Donnell Quality Engineering Manager Oracle EMEA , Dublin, Ireland ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
> There's an important difference: one exposes Apache HTTP client in > HSEARCH's SPI, whereas the other just requires usage of Apache HTTP > client within one specific implementation. For users it doesn't change > much, but the latter is cleaner from HSEARCH's perspective. RestClient is not an interface, it's an implementation. There's no interface. So yes, we would just be exposing Apache HTTP Client on top of RestClient. > I'm not quite following on that. If people are in control of the > RestClient entirely, they can do whatever they want? As mentioned above, exposing RestClient is even worse than just exposing the Apache HTTP client. So I was suggesting to use a proper abstraction, namely our own interface, org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient. I suppose your remark concerned this paragraph: > On top of that, this solution would not allow multiple third-party > customizations to work well together (for instance AWS authentication > provided by us or a third party + some performance tweaking by the user)... > which is something the SPI we're planning in the PR could allow. If we did allow users to re-implement ElasticsearchClient, yes, one implementor would be able to do whatever he wants... if he re-implements it. He wouldn't be able to re-use other extensions. Take for example the AWS authentication. You're suggestion that we provide an alternative client allowing to connect to AWS. Fine, we do that, and users can use it. But what if a users wants AWS authentication *and* say, configure a proxy? Then he can't reuse our AWS client, since this client is just an implementation of ElasticsearchClient, and we don't want to expose anything related to Apache HTTP Client. So he must implement AWS authentication. Just to configure a proxy. The thing is, there are tons of things a user may want to do with Apache HTTP Client, and we can't possibly provide access to each and every option through an abstraction layer. So at *some* point, if we want to allow configuration (and in the case of an HTTP client, I'm afraid we have to), we'll have to expose internals. We just have to make sure this is done in a controlled way (expose as little as possible). Yoann Rodière Hibernate NoORM Team yo...@hibernate.org On 2 June 2017 at 12:26, Gunnar Morling wrote: > 2017-06-02 11:58 GMT+02:00 Yoann Rodiere : > >> Did you instead consider to just let users provide their custom > >> instance of org.elasticsearch.client.RestClient? It's still leaking an > >> implementation detail of Hibernate Search, but at least it's one > >> indirection less. > > > > > > The only way to add AWS authentication to the RestClient is to use Apache > > HTTP Client classes, so this solution would still bind users to the > > implementation details. We'd just shove it under the carpet :) > > There's an important difference: one exposes Apache HTTP client in > HSEARCH's SPI, whereas the other just requires usage of Apache HTTP > client within one specific implementation. For users it doesn't change > much, but the latter is cleaner from HSEARCH's perspective. > > > > Also, I'd argue this would be an even bigger implementation leak: at > least > > with the current solution with can switch to an alternative Elasticsearch > > client, as long as we still use Apache HTTP Client. If we expose > RestClient, > > we're stuck with it and whatever underlying technologies it uses. > > > > On the other hand, we could ask them to re-implement > > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient, > which is > > our own façade over the Elasticsearch client. But I find it a bit much > just > > to tweak some settings or to add a new authentication scheme... > > On top of that, this solution would not allow multiple third-party > > customizations to work well together (for instance AWS authentication > > provided by us or a third party + some performance tweaking by the > user)... > > which is something the SPI we're planning in the PR could allow. > > I'm not quite following on that. If people are in control of the > RestClient entirely, they can do whatever they want? > > > > > Yoann Rodière > > Hibernate NoORM Team > > yo...@hibernate.org > > > > On 2 June 2017 at 10:30, Sanne Grinovero wrote: > >> > >> On Fri, 2 Jun 2017, 08:56 Gunnar Morling, wrote: > >> > >> > Hi, > >> > > >> > I find the exposure of an implementation detail (usage of Apache HTTP > >> > client) of the Elasticsearch client a bit problematic. If they change > >> > this to another HTTP client, our SPI would break. > >> > > >> > >> Yes the very point of exposing that detail is the reason for this > thread. > >> > >> Still, our SPI being guaranteed only for a minor, that gives a lot of > >> flexibility? > >> > >> The Elasticsearch client exposing this itself, I don't expect them to > >> switch implementation in a micro release to make some bugfix. If they > >> change it in a major or even minor version, we're ok to not support that > >> versio
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
2017-06-02 12:49 GMT+02:00 Yoann Rodiere :
>> There's an important difference: one exposes Apache HTTP client in
>> HSEARCH's SPI, whereas the other just requires usage of Apache HTTP
>> client within one specific implementation. For users it doesn't change
>> much, but the latter is cleaner from HSEARCH's perspective.
>
> RestClient is not an interface, it's an implementation. There's no
> interface. So yes, we would just be exposing Apache HTTP Client on top of
> RestClient.
>
Here's what I'd have done:
package org.hibernate.search.elasticsearch.client.spi;
public interface RestClientFactory {
RestClient buildRestClient(SomeContext ctx);
}
There's no exposure of HTTP Client in this SPI. Yes, if people need to
customize the HTTP client to be used by the returned RestClient
instance, they'll naturally depend on that. But this SPI isn't tied to
such detail of how RestClient works - if ES folks decided to use
OkHttp instead, our SPI contract won't be affected (of course user's
implementations will need to change if they were customizing the HTTP
client before).
>> I'm not quite following on that. If people are in control of the
>> RestClient entirely, they can do whatever they want?
>
> As mentioned above, exposing RestClient is even worse than just exposing the
> Apache HTTP client.
I don't think it's worse, it's better actually. It just exposes our
direct dependency in the SPI and not any of its details.
> So I was suggesting to use a proper abstraction, namely our own interface,
> org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient.
>
> I suppose your remark concerned this paragraph:
>
>> On top of that, this solution would not allow multiple third-party
>> customizations to work well together (for instance AWS authentication
>> provided by us or a third party + some performance tweaking by the
>> user)...
>> which is something the SPI we're planning in the PR could allow.
>
> If we did allow users to re-implement ElasticsearchClient, yes, one
> implementor would be able to do whatever he wants... if he re-implements it.
> He wouldn't be able to re-use other extensions.
> Take for example the AWS authentication. You're suggestion that we provide
> an alternative client allowing to connect to AWS. Fine, we do that, and
> users can use it. But what if a users wants AWS authentication and say,
> configure a proxy? Then he can't reuse our AWS client, since this client is
> just an implementation of ElasticsearchClient, and we don't want to expose
> anything related to Apache HTTP Client. So he must implement AWS
> authentication. Just to configure a proxy.
>
> The thing is, there are tons of things a user may want to do with Apache
> HTTP Client, and we can't possibly provide access to each and every option
> through an abstraction layer.
Right, hence I wouldn't bother to do that in the first place. Just let
users customize how RestClient is instantiated. Let's them do all they
want. We can provide examples which show how to do the AWS signing
etc.
> So at some point, if we want to allow
> configuration (and in the case of an HTTP client, I'm afraid we have to),
> we'll have to expose internals. We just have to make sure this is done in a
> controlled way (expose as little as possible).
>
>
>
> Yoann Rodière
> Hibernate NoORM Team
> yo...@hibernate.org
>
> On 2 June 2017 at 12:26, Gunnar Morling wrote:
>>
>> 2017-06-02 11:58 GMT+02:00 Yoann Rodiere :
>> >> Did you instead consider to just let users provide their custom
>> >> instance of org.elasticsearch.client.RestClient? It's still leaking an
>> >> implementation detail of Hibernate Search, but at least it's one
>> >> indirection less.
>> >
>> >
>> > The only way to add AWS authentication to the RestClient is to use
>> > Apache
>> > HTTP Client classes, so this solution would still bind users to the
>> > implementation details. We'd just shove it under the carpet :)
>>
>> There's an important difference: one exposes Apache HTTP client in
>> HSEARCH's SPI, whereas the other just requires usage of Apache HTTP
>> client within one specific implementation. For users it doesn't change
>> much, but the latter is cleaner from HSEARCH's perspective.
>> >
>> > Also, I'd argue this would be an even bigger implementation leak: at
>> > least
>> > with the current solution with can switch to an alternative
>> > Elasticsearch
>> > client, as long as we still use Apache HTTP Client. If we expose
>> > RestClient,
>> > we're stuck with it and whatever underlying technologies it uses.
>> >
>> > On the other hand, we could ask them to re-implement
>> > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient,
>> > which is
>> > our own façade over the Elasticsearch client. But I find it a bit much
>> > just
>> > to tweak some settings or to add a new authentication scheme...
>> > On top of that, this solution would not allow multiple third-party
>> > customizations to work well together (for instance AWS authentication
>
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
2017-06-02 12:29 GMT+02:00 Sanne Grinovero : > On 2 June 2017 at 10:58, Yoann Rodiere wrote: >>> Did you instead consider to just let users provide their custom >>> instance of org.elasticsearch.client.RestClient? It's still leaking an >>> implementation detail of Hibernate Search, but at least it's one >>> indirection less. >> >> >> The only way to add AWS authentication to the RestClient is to use Apache >> HTTP Client classes, so this solution would still bind users to the >> implementation details. We'd just shove it under the carpet :) >> >> Also, I'd argue this would be an even bigger implementation leak: at least >> with the current solution with can switch to an alternative Elasticsearch >> client, as long as we still use Apache HTTP Client. If we expose RestClient, >> we're stuck with it and whatever underlying technologies it uses. >> >> On the other hand, we could ask them to re-implement >> org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient, which is >> our own façade over the Elasticsearch client. But I find it a bit much just >> to tweak some settings or to add a new authentication scheme... >> On top of that, this solution would not allow multiple third-party >> customizations to work well together (for instance AWS authentication >> provided by us or a third party + some performance tweaking by the user)... >> which is something the SPI we're planning in the PR could allow. > > +1 > > I think you nailed it design wise. > > Gunnar, I have the impression we're simply not aligned on the definition of > SPI. > > How to do custom request signing on a *specific platform* (AWS) with a > *specific client* (this ES client using the Apache HTTP client) is > going to need a specific adaptor; the good news is that this is going > to be isolated so the main user application code - entities and > business logic - won't be affected. Sure, I don't see where either proposal is different in that regard? > That's SPI by the book and we're not giving guarantees of backwards > compatibility beyond a minor. > > We know the Elasticsearch driver is using the Apache HTTP client and > this won't change overnight; if they happen to change it, we'll change > in a new minor release and people will have to adapt. Understood that this is your rule. I don't find it a good one, though. IMO, minor releases shouldn't break APIs nor SPIs for that matter. YMMV. > > Too bad but it's practical. Let me also point out a drawback of your > alternative solution: the user's code for those we re-implement the > driver would not break but only if they fail to upgrade the driver, > which is possibly another pandora box. I don't follow. WDYM by "re-implementing the driver"? All I suggested is letting people instantiate their own RestClient. > > If you feel strongly about it I'm ok to promote the > `DefaultElasticsearchClientFactory` to SPI to give some more options > but I see no much value in that. Send a PR if you want it ;) I don't feel strongly about it - you asked for feedback on the proposal, so that's what I gave. I'd personally avoid to tie our SPIs to implementation specifics of dependencies. Nothing more, nothing less. > > Thanks, > Sanne > >> >> Yoann Rodière >> Hibernate NoORM Team >> yo...@hibernate.org >> >> On 2 June 2017 at 10:30, Sanne Grinovero wrote: >>> >>> On Fri, 2 Jun 2017, 08:56 Gunnar Morling, wrote: >>> >>> > Hi, >>> > >>> > I find the exposure of an implementation detail (usage of Apache HTTP >>> > client) of the Elasticsearch client a bit problematic. If they change >>> > this to another HTTP client, our SPI would break. >>> > >>> >>> Yes the very point of exposing that detail is the reason for this thread. >>> >>> Still, our SPI being guaranteed only for a minor, that gives a lot of >>> flexibility? >>> >>> The Elasticsearch client exposing this itself, I don't expect them to >>> switch implementation in a micro release to make some bugfix. If they >>> change it in a major or even minor version, we're ok to not support that >>> version until our next minor. >>> >>> >>> >>> > Did you instead consider to just let users provide their custom >>> > instance of org.elasticsearch.client.RestClient? It's still leaking an >>> > implementation detail of Hibernate Search, but at least it's one >>> > indirection less. >>> > >>> > People wishing to have a custom RestClient would have to implement a >>> > few more bits themselves (the logic from >>> > DefaultElasticsearchClientFactory#customizeHttpClientConfig()), but >>> > I'd find that acceptable for the sake of a less detail-exposing SPI, >>> > plus it grants more flexibility in terms of configuring the >>> > RestClient. >>> > >>> >>> >>> N.B. The client factory already is a Service so any advanced user already >>> can override it. >>> >>> We want to make it easier for cloud users. Focusing on AWS now as we've >>> had >>> user requests for this - not least our own CI - but I'd expect other >>> clouds >>> to have similar features (today or tom
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
> There's no exposure of HTTP Client in this SPI. Yes, if people need to
> customize the HTTP client to be used by the returned RestClient
> instance, they'll naturally depend on that. But this SPI isn't tied to
> such detail of how RestClient works - if ES folks decided to use
> OkHttp instead, our SPI contract won't be affected (of course user's
> implementations will need to change if they were customizing the HTTP
> client before).
I would feel dishonest arguing this to users... Frankly, there's little
point in returning a custom RestClient without customizing stuff related to
Apache HTTP Client. See RestClientBuilder: apart from methods tied to
Apache HTTP Client, and from options already provided by Hibernate Search,
you only have access to these:
- org.elasticsearch.client.RestClientBuilder.setDefaultHeaders(Header[])
-
org.elasticsearch.client.RestClientBuilder.setFailureListener(FailureListener)
- org.elasticsearch.client.RestClientBuilder.setPathPrefix(String)
... and that's all.
So yes, this SPI doesn't have a *direct* dependency to Apache HTTP Client,
but any practical use of it will have one. At the end of the day, that's
what really matters, right?
Yoann Rodière
Hibernate NoORM Team
yo...@hibernate.org
On 2 June 2017 at 14:25, Gunnar Morling wrote:
> 2017-06-02 12:49 GMT+02:00 Yoann Rodiere :
> >> There's an important difference: one exposes Apache HTTP client in
> >> HSEARCH's SPI, whereas the other just requires usage of Apache HTTP
> >> client within one specific implementation. For users it doesn't change
> >> much, but the latter is cleaner from HSEARCH's perspective.
> >
> > RestClient is not an interface, it's an implementation. There's no
> > interface. So yes, we would just be exposing Apache HTTP Client on top of
> > RestClient.
> >
>
> Here's what I'd have done:
>
> package org.hibernate.search.elasticsearch.client.spi;
>
> public interface RestClientFactory {
> RestClient buildRestClient(SomeContext ctx);
> }
>
> There's no exposure of HTTP Client in this SPI. Yes, if people need to
> customize the HTTP client to be used by the returned RestClient
> instance, they'll naturally depend on that. But this SPI isn't tied to
> such detail of how RestClient works - if ES folks decided to use
> OkHttp instead, our SPI contract won't be affected (of course user's
> implementations will need to change if they were customizing the HTTP
> client before).
>
> >> I'm not quite following on that. If people are in control of the
> >> RestClient entirely, they can do whatever they want?
> >
> > As mentioned above, exposing RestClient is even worse than just exposing
> the
> > Apache HTTP client.
>
> I don't think it's worse, it's better actually. It just exposes our
> direct dependency in the SPI and not any of its details.
>
> > So I was suggesting to use a proper abstraction, namely our own
> interface,
> > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient.
> >
> > I suppose your remark concerned this paragraph:
> >
> >> On top of that, this solution would not allow multiple third-party
> >> customizations to work well together (for instance AWS authentication
> >> provided by us or a third party + some performance tweaking by the
> >> user)...
> >> which is something the SPI we're planning in the PR could allow.
> >
> > If we did allow users to re-implement ElasticsearchClient, yes, one
> > implementor would be able to do whatever he wants... if he re-implements
> it.
> > He wouldn't be able to re-use other extensions.
> > Take for example the AWS authentication. You're suggestion that we
> provide
> > an alternative client allowing to connect to AWS. Fine, we do that, and
> > users can use it. But what if a users wants AWS authentication and say,
> > configure a proxy? Then he can't reuse our AWS client, since this client
> is
> > just an implementation of ElasticsearchClient, and we don't want to
> expose
> > anything related to Apache HTTP Client. So he must implement AWS
> > authentication. Just to configure a proxy.
> >
> > The thing is, there are tons of things a user may want to do with Apache
> > HTTP Client, and we can't possibly provide access to each and every
> option
> > through an abstraction layer.
>
> Right, hence I wouldn't bother to do that in the first place. Just let
> users customize how RestClient is instantiated. Let's them do all they
> want. We can provide examples which show how to do the AWS signing
> etc.
>
> > So at some point, if we want to allow
> > configuration (and in the case of an HTTP client, I'm afraid we have to),
> > we'll have to expose internals. We just have to make sure this is done
> in a
> > controlled way (expose as little as possible).
> >
> >
> >
> > Yoann Rodière
> > Hibernate NoORM Team
> > yo...@hibernate.org
> >
> > On 2 June 2017 at 12:26, Gunnar Morling wrote:
> >>
> >> 2017-06-02 11:58 GMT+02:00 Yoann Rodiere :
> >> >> Did you instead consider to just let users provide their custo
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
Sure, as I said, an implementor of that SPI will very likely deal with
the HTTP client. But our SPI isn't tainted by that.
I also don't see much value in re-defining all the options from
RestClientBuilder. ElasticsearchHttpClientConfigurer resembles
HttpClientConfigCallback. Would you also re-define
RequestConfigCallback? What if new options get added to
RestClientBuilder? What's the benefit of this catch-up game?
2017-06-02 14:47 GMT+02:00 Yoann Rodiere :
>> There's no exposure of HTTP Client in this SPI. Yes, if people need to
>> customize the HTTP client to be used by the returned RestClient
>> instance, they'll naturally depend on that. But this SPI isn't tied to
>> such detail of how RestClient works - if ES folks decided to use
>> OkHttp instead, our SPI contract won't be affected (of course user's
>> implementations will need to change if they were customizing the HTTP
>> client before).
>
> I would feel dishonest arguing this to users... Frankly, there's little
> point in returning a custom RestClient without customizing stuff related to
> Apache HTTP Client. See RestClientBuilder: apart from methods tied to Apache
> HTTP Client, and from options already provided by Hibernate Search, you only
> have access to these:
>
> org.elasticsearch.client.RestClientBuilder.setDefaultHeaders(Header[])
> org.elasticsearch.client.RestClientBuilder.setFailureListener(FailureListener)
> org.elasticsearch.client.RestClientBuilder.setPathPrefix(String)
>
> ... and that's all.
>
> So yes, this SPI doesn't have a direct dependency to Apache HTTP Client, but
> any practical use of it will have one. At the end of the day, that's what
> really matters, right?
>
> Yoann Rodière
> Hibernate NoORM Team
> yo...@hibernate.org
>
> On 2 June 2017 at 14:25, Gunnar Morling wrote:
>>
>> 2017-06-02 12:49 GMT+02:00 Yoann Rodiere :
>> >> There's an important difference: one exposes Apache HTTP client in
>> >> HSEARCH's SPI, whereas the other just requires usage of Apache HTTP
>> >> client within one specific implementation. For users it doesn't change
>> >> much, but the latter is cleaner from HSEARCH's perspective.
>> >
>> > RestClient is not an interface, it's an implementation. There's no
>> > interface. So yes, we would just be exposing Apache HTTP Client on top
>> > of
>> > RestClient.
>> >
>>
>> Here's what I'd have done:
>>
>> package org.hibernate.search.elasticsearch.client.spi;
>>
>> public interface RestClientFactory {
>> RestClient buildRestClient(SomeContext ctx);
>> }
>>
>> There's no exposure of HTTP Client in this SPI. Yes, if people need to
>> customize the HTTP client to be used by the returned RestClient
>> instance, they'll naturally depend on that. But this SPI isn't tied to
>> such detail of how RestClient works - if ES folks decided to use
>> OkHttp instead, our SPI contract won't be affected (of course user's
>> implementations will need to change if they were customizing the HTTP
>> client before).
>>
>> >> I'm not quite following on that. If people are in control of the
>> >> RestClient entirely, they can do whatever they want?
>> >
>> > As mentioned above, exposing RestClient is even worse than just exposing
>> > the
>> > Apache HTTP client.
>>
>> I don't think it's worse, it's better actually. It just exposes our
>> direct dependency in the SPI and not any of its details.
>>
>> > So I was suggesting to use a proper abstraction, namely our own
>> > interface,
>> > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient.
>> >
>> > I suppose your remark concerned this paragraph:
>> >
>> >> On top of that, this solution would not allow multiple third-party
>> >> customizations to work well together (for instance AWS authentication
>> >> provided by us or a third party + some performance tweaking by the
>> >> user)...
>> >> which is something the SPI we're planning in the PR could allow.
>> >
>> > If we did allow users to re-implement ElasticsearchClient, yes, one
>> > implementor would be able to do whatever he wants... if he re-implements
>> > it.
>> > He wouldn't be able to re-use other extensions.
>> > Take for example the AWS authentication. You're suggestion that we
>> > provide
>> > an alternative client allowing to connect to AWS. Fine, we do that, and
>> > users can use it. But what if a users wants AWS authentication and say,
>> > configure a proxy? Then he can't reuse our AWS client, since this client
>> > is
>> > just an implementation of ElasticsearchClient, and we don't want to
>> > expose
>> > anything related to Apache HTTP Client. So he must implement AWS
>> > authentication. Just to configure a proxy.
>> >
>> > The thing is, there are tons of things a user may want to do with Apache
>> > HTTP Client, and we can't possibly provide access to each and every
>> > option
>> > through an abstraction layer.
>>
>> Right, hence I wouldn't bother to do that in the first place. Just let
>> users customize how RestClient is instantiated. Let's them do all they
>> want
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
I made a mistake, Header is an Apache HTTP Client type and FailureListener
depends on one. So we're down to one useful customization, which we should
probably support as an Hibernate Search option by the way:
- org.elasticsearch.client.RestClientBuilder.setPathPrefix(String)
Yoann Rodière
Hibernate NoORM Team
yo...@hibernate.org
On 2 June 2017 at 14:47, Yoann Rodiere wrote:
> > There's no exposure of HTTP Client in this SPI. Yes, if people need to
> > customize the HTTP client to be used by the returned RestClient
> > instance, they'll naturally depend on that. But this SPI isn't tied to
> > such detail of how RestClient works - if ES folks decided to use
> > OkHttp instead, our SPI contract won't be affected (of course user's
> > implementations will need to change if they were customizing the HTTP
> > client before).
>
> I would feel dishonest arguing this to users... Frankly, there's little
> point in returning a custom RestClient without customizing stuff related to
> Apache HTTP Client. See RestClientBuilder: apart from methods tied to
> Apache HTTP Client, and from options already provided by Hibernate Search,
> you only have access to these:
>
>- org.elasticsearch.client.RestClientBuilder.
>setDefaultHeaders(Header[])
>- org.elasticsearch.client.RestClientBuilder.setFailureListener(
>FailureListener)
>- org.elasticsearch.client.RestClientBuilder.setPathPrefix(String)
>
> ... and that's all.
>
> So yes, this SPI doesn't have a *direct* dependency to Apache HTTP
> Client, but any practical use of it will have one. At the end of the day,
> that's what really matters, right?
>
> Yoann Rodière
> Hibernate NoORM Team
> yo...@hibernate.org
>
> On 2 June 2017 at 14:25, Gunnar Morling wrote:
>
>> 2017-06-02 12:49 GMT+02:00 Yoann Rodiere :
>> >> There's an important difference: one exposes Apache HTTP client in
>> >> HSEARCH's SPI, whereas the other just requires usage of Apache HTTP
>> >> client within one specific implementation. For users it doesn't change
>> >> much, but the latter is cleaner from HSEARCH's perspective.
>> >
>> > RestClient is not an interface, it's an implementation. There's no
>> > interface. So yes, we would just be exposing Apache HTTP Client on top
>> of
>> > RestClient.
>> >
>>
>> Here's what I'd have done:
>>
>> package org.hibernate.search.elasticsearch.client.spi;
>>
>> public interface RestClientFactory {
>> RestClient buildRestClient(SomeContext ctx);
>> }
>>
>> There's no exposure of HTTP Client in this SPI. Yes, if people need to
>> customize the HTTP client to be used by the returned RestClient
>> instance, they'll naturally depend on that. But this SPI isn't tied to
>> such detail of how RestClient works - if ES folks decided to use
>> OkHttp instead, our SPI contract won't be affected (of course user's
>> implementations will need to change if they were customizing the HTTP
>> client before).
>>
>> >> I'm not quite following on that. If people are in control of the
>> >> RestClient entirely, they can do whatever they want?
>> >
>> > As mentioned above, exposing RestClient is even worse than just
>> exposing the
>> > Apache HTTP client.
>>
>> I don't think it's worse, it's better actually. It just exposes our
>> direct dependency in the SPI and not any of its details.
>>
>> > So I was suggesting to use a proper abstraction, namely our own
>> interface,
>> > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient.
>> >
>> > I suppose your remark concerned this paragraph:
>> >
>> >> On top of that, this solution would not allow multiple third-party
>> >> customizations to work well together (for instance AWS authentication
>> >> provided by us or a third party + some performance tweaking by the
>> >> user)...
>> >> which is something the SPI we're planning in the PR could allow.
>> >
>> > If we did allow users to re-implement ElasticsearchClient, yes, one
>> > implementor would be able to do whatever he wants... if he
>> re-implements it.
>> > He wouldn't be able to re-use other extensions.
>> > Take for example the AWS authentication. You're suggestion that we
>> provide
>> > an alternative client allowing to connect to AWS. Fine, we do that, and
>> > users can use it. But what if a users wants AWS authentication and say,
>> > configure a proxy? Then he can't reuse our AWS client, since this
>> client is
>> > just an implementation of ElasticsearchClient, and we don't want to
>> expose
>> > anything related to Apache HTTP Client. So he must implement AWS
>> > authentication. Just to configure a proxy.
>> >
>> > The thing is, there are tons of things a user may want to do with Apache
>> > HTTP Client, and we can't possibly provide access to each and every
>> option
>> > through an abstraction layer.
>>
>> Right, hence I wouldn't bother to do that in the first place. Just let
>> users customize how RestClient is instantiated. Let's them do all they
>> want. We can provide examples which show how to do the AWS signin
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
> What's the benefit of this catch-up game?
Not tainting our SPI with RestClient :)
Yoann Rodière
Hibernate NoORM Team
yo...@hibernate.org
On 2 June 2017 at 14:59, Gunnar Morling wrote:
> Sure, as I said, an implementor of that SPI will very likely deal with
> the HTTP client. But our SPI isn't tainted by that.
>
> I also don't see much value in re-defining all the options from
> RestClientBuilder. ElasticsearchHttpClientConfigurer resembles
> HttpClientConfigCallback. Would you also re-define
> RequestConfigCallback? What if new options get added to
> RestClientBuilder? What's the benefit of this catch-up game?
>
>
>
>
> 2017-06-02 14:47 GMT+02:00 Yoann Rodiere :
> >> There's no exposure of HTTP Client in this SPI. Yes, if people need to
> >> customize the HTTP client to be used by the returned RestClient
> >> instance, they'll naturally depend on that. But this SPI isn't tied to
> >> such detail of how RestClient works - if ES folks decided to use
> >> OkHttp instead, our SPI contract won't be affected (of course user's
> >> implementations will need to change if they were customizing the HTTP
> >> client before).
> >
> > I would feel dishonest arguing this to users... Frankly, there's little
> > point in returning a custom RestClient without customizing stuff related
> to
> > Apache HTTP Client. See RestClientBuilder: apart from methods tied to
> Apache
> > HTTP Client, and from options already provided by Hibernate Search, you
> only
> > have access to these:
> >
> > org.elasticsearch.client.RestClientBuilder.setDefaultHeaders(Header[])
> > org.elasticsearch.client.RestClientBuilder.setFailureListener(
> FailureListener)
> > org.elasticsearch.client.RestClientBuilder.setPathPrefix(String)
> >
> > ... and that's all.
> >
> > So yes, this SPI doesn't have a direct dependency to Apache HTTP Client,
> but
> > any practical use of it will have one. At the end of the day, that's what
> > really matters, right?
> >
> > Yoann Rodière
> > Hibernate NoORM Team
> > yo...@hibernate.org
> >
> > On 2 June 2017 at 14:25, Gunnar Morling wrote:
> >>
> >> 2017-06-02 12:49 GMT+02:00 Yoann Rodiere :
> >> >> There's an important difference: one exposes Apache HTTP client in
> >> >> HSEARCH's SPI, whereas the other just requires usage of Apache HTTP
> >> >> client within one specific implementation. For users it doesn't
> change
> >> >> much, but the latter is cleaner from HSEARCH's perspective.
> >> >
> >> > RestClient is not an interface, it's an implementation. There's no
> >> > interface. So yes, we would just be exposing Apache HTTP Client on top
> >> > of
> >> > RestClient.
> >> >
> >>
> >> Here's what I'd have done:
> >>
> >> package org.hibernate.search.elasticsearch.client.spi;
> >>
> >> public interface RestClientFactory {
> >> RestClient buildRestClient(SomeContext ctx);
> >> }
> >>
> >> There's no exposure of HTTP Client in this SPI. Yes, if people need to
> >> customize the HTTP client to be used by the returned RestClient
> >> instance, they'll naturally depend on that. But this SPI isn't tied to
> >> such detail of how RestClient works - if ES folks decided to use
> >> OkHttp instead, our SPI contract won't be affected (of course user's
> >> implementations will need to change if they were customizing the HTTP
> >> client before).
> >>
> >> >> I'm not quite following on that. If people are in control of the
> >> >> RestClient entirely, they can do whatever they want?
> >> >
> >> > As mentioned above, exposing RestClient is even worse than just
> exposing
> >> > the
> >> > Apache HTTP client.
> >>
> >> I don't think it's worse, it's better actually. It just exposes our
> >> direct dependency in the SPI and not any of its details.
> >>
> >> > So I was suggesting to use a proper abstraction, namely our own
> >> > interface,
> >> > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient.
> >> >
> >> > I suppose your remark concerned this paragraph:
> >> >
> >> >> On top of that, this solution would not allow multiple third-party
> >> >> customizations to work well together (for instance AWS authentication
> >> >> provided by us or a third party + some performance tweaking by the
> >> >> user)...
> >> >> which is something the SPI we're planning in the PR could allow.
> >> >
> >> > If we did allow users to re-implement ElasticsearchClient, yes, one
> >> > implementor would be able to do whatever he wants... if he
> re-implements
> >> > it.
> >> > He wouldn't be able to re-use other extensions.
> >> > Take for example the AWS authentication. You're suggestion that we
> >> > provide
> >> > an alternative client allowing to connect to AWS. Fine, we do that,
> and
> >> > users can use it. But what if a users wants AWS authentication and
> say,
> >> > configure a proxy? Then he can't reuse our AWS client, since this
> client
> >> > is
> >> > just an implementation of ElasticsearchClient, and we don't want to
> >> > expose
> >> > anything related to Apache HTTP Client. So he must im
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
I reckon you are making fun of me. Anyways, I've had my say :)
2017-06-02 15:03 GMT+02:00 Yoann Rodiere :
>> What's the benefit of this catch-up game?
>
> Not tainting our SPI with RestClient :)
>
> Yoann Rodière
> Hibernate NoORM Team
> yo...@hibernate.org
>
> On 2 June 2017 at 14:59, Gunnar Morling wrote:
>>
>> Sure, as I said, an implementor of that SPI will very likely deal with
>> the HTTP client. But our SPI isn't tainted by that.
>>
>> I also don't see much value in re-defining all the options from
>> RestClientBuilder. ElasticsearchHttpClientConfigurer resembles
>> HttpClientConfigCallback. Would you also re-define
>> RequestConfigCallback? What if new options get added to
>> RestClientBuilder? What's the benefit of this catch-up game?
>>
>>
>>
>>
>> 2017-06-02 14:47 GMT+02:00 Yoann Rodiere :
>> >> There's no exposure of HTTP Client in this SPI. Yes, if people need to
>> >> customize the HTTP client to be used by the returned RestClient
>> >> instance, they'll naturally depend on that. But this SPI isn't tied to
>> >> such detail of how RestClient works - if ES folks decided to use
>> >> OkHttp instead, our SPI contract won't be affected (of course user's
>> >> implementations will need to change if they were customizing the HTTP
>> >> client before).
>> >
>> > I would feel dishonest arguing this to users... Frankly, there's little
>> > point in returning a custom RestClient without customizing stuff related
>> > to
>> > Apache HTTP Client. See RestClientBuilder: apart from methods tied to
>> > Apache
>> > HTTP Client, and from options already provided by Hibernate Search, you
>> > only
>> > have access to these:
>> >
>> > org.elasticsearch.client.RestClientBuilder.setDefaultHeaders(Header[])
>> >
>> > org.elasticsearch.client.RestClientBuilder.setFailureListener(FailureListener)
>> > org.elasticsearch.client.RestClientBuilder.setPathPrefix(String)
>> >
>> > ... and that's all.
>> >
>> > So yes, this SPI doesn't have a direct dependency to Apache HTTP Client,
>> > but
>> > any practical use of it will have one. At the end of the day, that's
>> > what
>> > really matters, right?
>> >
>> > Yoann Rodière
>> > Hibernate NoORM Team
>> > yo...@hibernate.org
>> >
>> > On 2 June 2017 at 14:25, Gunnar Morling wrote:
>> >>
>> >> 2017-06-02 12:49 GMT+02:00 Yoann Rodiere :
>> >> >> There's an important difference: one exposes Apache HTTP client in
>> >> >> HSEARCH's SPI, whereas the other just requires usage of Apache HTTP
>> >> >> client within one specific implementation. For users it doesn't
>> >> >> change
>> >> >> much, but the latter is cleaner from HSEARCH's perspective.
>> >> >
>> >> > RestClient is not an interface, it's an implementation. There's no
>> >> > interface. So yes, we would just be exposing Apache HTTP Client on
>> >> > top
>> >> > of
>> >> > RestClient.
>> >> >
>> >>
>> >> Here's what I'd have done:
>> >>
>> >> package org.hibernate.search.elasticsearch.client.spi;
>> >>
>> >> public interface RestClientFactory {
>> >> RestClient buildRestClient(SomeContext ctx);
>> >> }
>> >>
>> >> There's no exposure of HTTP Client in this SPI. Yes, if people need to
>> >> customize the HTTP client to be used by the returned RestClient
>> >> instance, they'll naturally depend on that. But this SPI isn't tied to
>> >> such detail of how RestClient works - if ES folks decided to use
>> >> OkHttp instead, our SPI contract won't be affected (of course user's
>> >> implementations will need to change if they were customizing the HTTP
>> >> client before).
>> >>
>> >> >> I'm not quite following on that. If people are in control of the
>> >> >> RestClient entirely, they can do whatever they want?
>> >> >
>> >> > As mentioned above, exposing RestClient is even worse than just
>> >> > exposing
>> >> > the
>> >> > Apache HTTP client.
>> >>
>> >> I don't think it's worse, it's better actually. It just exposes our
>> >> direct dependency in the SPI and not any of its details.
>> >>
>> >> > So I was suggesting to use a proper abstraction, namely our own
>> >> > interface,
>> >> > org.hibernate.search.elasticsearch.client.impl.ElasticsearchClient.
>> >> >
>> >> > I suppose your remark concerned this paragraph:
>> >> >
>> >> >> On top of that, this solution would not allow multiple third-party
>> >> >> customizations to work well together (for instance AWS
>> >> >> authentication
>> >> >> provided by us or a third party + some performance tweaking by the
>> >> >> user)...
>> >> >> which is something the SPI we're planning in the PR could allow.
>> >> >
>> >> > If we did allow users to re-implement ElasticsearchClient, yes, one
>> >> > implementor would be able to do whatever he wants... if he
>> >> > re-implements
>> >> > it.
>> >> > He wouldn't be able to re-use other extensions.
>> >> > Take for example the AWS authentication. You're suggestion that we
>> >> > provide
>> >> > an alternative client allowing to connect to AWS. Fine, we do that,
>> >> > and
>> >> > users can use it. But what if
Re: [hibernate-dev] Introducing a new API in Hibernate Search to "sign" HTTP requests to Elasticsearch
> Anyways, I've had my say :)
And I'm grateful you took the time to do it. I guess we both would have
preferred to agree on the same solution, but... at least we now have
something to process.
Yoann Rodière
Hibernate NoORM Team
yo...@hibernate.org
On 2 June 2017 at 15:11, Gunnar Morling wrote:
> I reckon you are making fun of me. Anyways, I've had my say :)
>
> 2017-06-02 15:03 GMT+02:00 Yoann Rodiere :
> >> What's the benefit of this catch-up game?
> >
> > Not tainting our SPI with RestClient :)
> >
> > Yoann Rodière
> > Hibernate NoORM Team
> > yo...@hibernate.org
> >
> > On 2 June 2017 at 14:59, Gunnar Morling wrote:
> >>
> >> Sure, as I said, an implementor of that SPI will very likely deal with
> >> the HTTP client. But our SPI isn't tainted by that.
> >>
> >> I also don't see much value in re-defining all the options from
> >> RestClientBuilder. ElasticsearchHttpClientConfigurer resembles
> >> HttpClientConfigCallback. Would you also re-define
> >> RequestConfigCallback? What if new options get added to
> >> RestClientBuilder? What's the benefit of this catch-up game?
> >>
> >>
> >>
> >>
> >> 2017-06-02 14:47 GMT+02:00 Yoann Rodiere :
> >> >> There's no exposure of HTTP Client in this SPI. Yes, if people need
> to
> >> >> customize the HTTP client to be used by the returned RestClient
> >> >> instance, they'll naturally depend on that. But this SPI isn't tied
> to
> >> >> such detail of how RestClient works - if ES folks decided to use
> >> >> OkHttp instead, our SPI contract won't be affected (of course user's
> >> >> implementations will need to change if they were customizing the HTTP
> >> >> client before).
> >> >
> >> > I would feel dishonest arguing this to users... Frankly, there's
> little
> >> > point in returning a custom RestClient without customizing stuff
> related
> >> > to
> >> > Apache HTTP Client. See RestClientBuilder: apart from methods tied to
> >> > Apache
> >> > HTTP Client, and from options already provided by Hibernate Search,
> you
> >> > only
> >> > have access to these:
> >> >
> >> > org.elasticsearch.client.RestClientBuilder.
> setDefaultHeaders(Header[])
> >> >
> >> > org.elasticsearch.client.RestClientBuilder.setFailureListener(
> FailureListener)
> >> > org.elasticsearch.client.RestClientBuilder.setPathPrefix(String)
> >> >
> >> > ... and that's all.
> >> >
> >> > So yes, this SPI doesn't have a direct dependency to Apache HTTP
> Client,
> >> > but
> >> > any practical use of it will have one. At the end of the day, that's
> >> > what
> >> > really matters, right?
> >> >
> >> > Yoann Rodière
> >> > Hibernate NoORM Team
> >> > yo...@hibernate.org
> >> >
> >> > On 2 June 2017 at 14:25, Gunnar Morling wrote:
> >> >>
> >> >> 2017-06-02 12:49 GMT+02:00 Yoann Rodiere :
> >> >> >> There's an important difference: one exposes Apache HTTP client in
> >> >> >> HSEARCH's SPI, whereas the other just requires usage of Apache
> HTTP
> >> >> >> client within one specific implementation. For users it doesn't
> >> >> >> change
> >> >> >> much, but the latter is cleaner from HSEARCH's perspective.
> >> >> >
> >> >> > RestClient is not an interface, it's an implementation. There's no
> >> >> > interface. So yes, we would just be exposing Apache HTTP Client on
> >> >> > top
> >> >> > of
> >> >> > RestClient.
> >> >> >
> >> >>
> >> >> Here's what I'd have done:
> >> >>
> >> >> package org.hibernate.search.elasticsearch.client.spi;
> >> >>
> >> >> public interface RestClientFactory {
> >> >> RestClient buildRestClient(SomeContext ctx);
> >> >> }
> >> >>
> >> >> There's no exposure of HTTP Client in this SPI. Yes, if people need
> to
> >> >> customize the HTTP client to be used by the returned RestClient
> >> >> instance, they'll naturally depend on that. But this SPI isn't tied
> to
> >> >> such detail of how RestClient works - if ES folks decided to use
> >> >> OkHttp instead, our SPI contract won't be affected (of course user's
> >> >> implementations will need to change if they were customizing the HTTP
> >> >> client before).
> >> >>
> >> >> >> I'm not quite following on that. If people are in control of the
> >> >> >> RestClient entirely, they can do whatever they want?
> >> >> >
> >> >> > As mentioned above, exposing RestClient is even worse than just
> >> >> > exposing
> >> >> > the
> >> >> > Apache HTTP client.
> >> >>
> >> >> I don't think it's worse, it's better actually. It just exposes our
> >> >> direct dependency in the SPI and not any of its details.
> >> >>
> >> >> > So I was suggesting to use a proper abstraction, namely our own
> >> >> > interface,
> >> >> > org.hibernate.search.elasticsearch.client.impl.
> ElasticsearchClient.
> >> >> >
> >> >> > I suppose your remark concerned this paragraph:
> >> >> >
> >> >> >> On top of that, this solution would not allow multiple third-party
> >> >> >> customizations to work well together (for instance AWS
> >> >> >> authentication
> >> >> >> provided by us or a third party + some performance tweaking by th
