Looks good to me but I'll defer to Christian since I know little about ES. Thanks for this Andrei! -- Michael Mior [email protected]
Le ven. 22 juin 2018 à 14:13, Andrei Sereda <[email protected]> a écrit : > CALCITE-2376 <https://issues.apache.org/jira/browse/CALCITE-2376> > > Please let me know if you agree with the plan > > On Fri, Jun 22, 2018 at 1:47 PM Christian Beikov < > [email protected]> > wrote: > > > The original reason for having separate adapters was because the ES > > Java-Client SDKs require different library versions which aren't binary > > compatible. Having separate modules just seemed to be the simplest > > solution. If you can make sure this is not going to be a problem for > > users, I'd be all for unifying the adapters. Changing the dependency and > > the schema factory name are IMO not problematic. > > > > > > Mit freundlichen Grüßen, > > ------------------------------------------------------------------------ > > *Christian Beikov* > > Am 22.06.2018 um 19:07 schrieb Andrei Sereda: > > > 1) If we go single (and separate) ES adapter route, people will have to > > > change their existing maven dependencies as well as ES schema > > configuration > > > (at least SchemaFactory name). I'm not sure if there are any explicit > (or > > > implicit) backwards compatibility policies in calcite. There are > (albeit) > > > small implications for end-user. > > > > > > On Fri, Jun 22, 2018 at 12:54 PM Michael Mior <[email protected]> > wrote: > > > > > >> 1) I personally would be open to this unless there's strong evidence > of > > use > > >> of the ES2 adapter. > > >> > > >> 2) Calcite already depends on Jackson in core and both ES modules, so > > this > > >> isn't a concern. > > >> -- > > >> Michael Mior > > >> [email protected] > > >> > > >> > > >> > > >> Le ven. 22 juin 2018 à 12:37, Andrei Sereda <[email protected]> a > écrit > > : > > >> > > >>> Some questions regarding this change: > > >>> > > >>> 1) Should one remove ES2 and ES5 adapters (maven modules) in favor of > > >>> single one: just ES ? This will be backwards incompatible change. Or > > keep > > >>> them as is and create a new module ? There is also quite a bit of ES > > >>> related code in calcite-core. > > >>> > > >>> 2) Since I need to create / parse JSON formats, ES adapter would have > > to > > >>> depend on some JSON library (most likely existing Jackson). Is that > > >>> acceptable ? > > >>> > > >>> > > >>> > > >>> On Fri, May 18, 2018 at 4:29 PM Andrei Sereda <[email protected]> > > wrote: > > >>> > > >>>> I believe this shouldn't be an issue with http client (contrary to > > >> native > > >>>> transport) > > >>>> > > >>>> On Fri, May 18, 2018, 16:16 Christian Beikov < > > >> [email protected] > > >>>> wrote: > > >>>> > > >>>>> That's mainly because the Java drivers changed in a way that made > > >>>>> impossible to use the same adapter. I might be wrong, but I think > the > > >>>>> ES5 adapter doesn't work with an ES2 server instance just like the > > ES2 > > >>>>> adapter doesn't work with an ES5+ server instance. > > >>>>> > > >>>>> If all of this could just go away, that would be great :) > > >>>>> > > >>>>> > > >>>>> Mit freundlichen Grüßen, > > >>>>> > > >> > ------------------------------------------------------------------------ > > >>>>> *Christian Beikov* > > >>>>> Am 18.05.2018 um 21:19 schrieb Andrei Sereda: > > >>>>>> Yes it should be, since it is just an http client (apache http). > > >>>>>> ElasticSearch Rest API (query API) didn't change much > > >>>>>> < > > >> > > > https://www.elastic.co/guide/en/elasticsearch/reference/5.0/breaking-changes-5.0.html > > >>>>>> . > > >>>>>> > > >>>>>> Next question would be : why there is a need in two separate > modules > > >>>>>> elasticsearch2 and elasticsearch5 > > >>>>>> > > >>>>>> On Fri, May 18, 2018 at 3:11 PM, Christian Beikov < > > >>>>>> [email protected]> wrote: > > >>>>>> > > >>>>>>> Hey Andrei, > > >>>>>>> > > >>>>>>> that would be awesome! Do you know by any chance if the low level > > >>>>> client > > >>>>>>> is also compatible with older ES versions? > > >>>>>>> > > >>>>>>> > > >>>>>>> Mit freundlichen Grüßen, > > >>>>>>> > > >> > ------------------------------------------------------------------------ > > >>>>>>> *Christian Beikov* > > >>>>>>> Am 18.05.2018 um 20:45 schrieb Andrei Sereda: > > >>>>>>> > > >>>>>>>> Hello, > > >>>>>>>> > > >>>>>>>> ES TransportClient is deprecated in 7.0 (to be removed > > >>>>>>>> <https://www.elastic.co/guide/en/elasticsearch/client/java-a > > >>>>>>>> pi/master/transport-client.html> > > >>>>>>>> in 8.0) in favor of http rest client(s). Would you consider a > > >>>>> contribution > > >>>>>>>> switching to Rest Low-Level Client > > >>>>>>>> <https://www.elastic.co/guide/en/elasticsearch/client/java-r > > >>>>>>>> est/current/java-rest-low.html>(which > > >>>>>>>> has much fewer dependencies) ? > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> Thanks, > > >>>>>>>> Andrei. > > >>>>>>>> > > >>>>>>>> > > >>>>> > > > > >
