metamodel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kasper Sørensen <i.am.kasper.soren...@gmail.com>
Subject Re: Elastic Search module - Progress update
Date Wed, 17 Sep 2014 19:21:12 GMT
Looks like good changes :-) Happy to co-develop some of this. I made a few
further remarks, but so far looking great!

2014-09-17 10:56 GMT+02:00 Alberto Rodriguez <arodriguez@stratio.com>:

> Hi Kasper,
>
> please see my notes on your PR comments, I've also pushed new changes to
> the repo.
>
> Kind regards,
>
> Alberto Rodríguez
>
>
> <http://www.stratio.com/>
> Avenida de Europa, 26. Ática 5. 3ª Planta
> 28224 Pozuelo de Alarcón, Madrid
> Tel: +34 91 352 59 42 // *@stratiobd <https://twitter.com/StratioBD>*
>
> 2014-09-17 9:45 GMT+02:00 Kasper Sørensen <i.am.kasper.sorensen@gmail.com
> >:
>
> > Hi Alberto,
> >
> > I added a bunch of remarks to the PR in github. None of them absolutely
> > critical to do before we merge it, but nice if you anyway want to spend
> > more time on it.
> >
> > Best regards
> > Kasper
> > Den 16/09/2014 23.01 skrev "Henry Saputra" <henry.saputra@gmail.com>:
> >
> > > Thanks Alberto, this should help review the patch
> > >
> > > - Henry
> > >
> > > On Tue, Sep 16, 2014 at 1:44 PM, Alberto Rodriguez <ardlema@gmail.com>
> > > wrote:
> > > > Hi Henry,
> > > >
> > > > I have just submitted a PR against the MM github mirror.
> > > >
> > > > Kind regards,
> > > >
> > > > Alberto Rodríguez
> > > >
> > > > 2014-09-16 19:01 GMT+02:00 Henry Saputra <henry.saputra@gmail.com>:
> > > >
> > > >> Alberto,
> > > >>
> > > >> Thanks for the ES module patch.
> > > >>
> > > >> Could you submit PR against Apache MetaModel github mirror?
> > > >> We may need to manually merge your fixes to ASF Git repo later but
> > > >> Github PR will help with reviews.
> > > >>
> > > >> - Henry
> > > >>
> > > >> On Tue, Sep 16, 2014 at 2:45 AM, Alberto Rodriguez
> > > >> <arodriguez@stratio.com> wrote:
> > > >> > Hi Kasper,
> > > >> >
> > > >> > thank you very much for your feedback!
> > > >> >
> > > >> > I have just modified the getMainSchemaName method to get the
> cluster
> > > name
> > > >> > from ES. Regarding the executeQuery method I commented it out
> > because
> > > >> some
> > > >> > tests were failing, anyway I will try to make this method work
and
> > get
> > > >> all
> > > >> > the benefits from the awesome elasticsearch java API.
> > > >> >
> > > >> > My apache username is: albertostratio
> > > >> >
> > > >> > Kind regards,
> > > >> >
> > > >> >
> > > >> > Alberto Rodríguez
> > > >> >
> > > >> >
> > > >> > <http://www.stratio.com/>
> > > >> > Avenida de Europa, 26. Ática 5. 3ª Planta
> > > >> > 28224 Pozuelo de Alarcón, Madrid
> > > >> > Tel: +34 91 352 59 42 // *@stratiobd <
> https://twitter.com/StratioBD
> > >*
> > > >> >
> > > >> > 2014-09-16 10:45 GMT+02:00 Kasper Sørensen <
> > > >> i.am.kasper.sorensen@gmail.com>:
> > > >> >
> > > >> >> Hi Alberto,
> > > >> >>
> > > >> >> Once again, thank you very much for this great work. I think
this
> > > >> module is
> > > >> >> looking 99% ready to be introduced into the main MetaModel
> > codebase.
> > > >> >>
> > > >> >> Here are my very minor remarks:
> > > >> >>
> > > >> >>
> > > >> >>    - The main schema name "ElasticSearchSchema" is hardcoded
and
> > not
> > > >> >>    necesarily what users would want. I was thinking if we
should
> > > either:
> > > >> >>    - Make it parameterizable in the constructor (default
value
> > would
> > > IMO
> > > >> >>       better be just "ElasticSearch" which is in line with
> > "MongoDB",
> > > >> >> "CouchDB"
> > > >> >>       etc.)
> > > >> >>       - Use the ES cluster name. This would give a more "native"
> > > >> feeling -
> > > >> >>       then we've mapped it to something that is recognizable
from
> > > the ES
> > > >> >>       connection.
> > > >> >>       - Or a combination of the above - make a constructor
name
> > > >> available,
> > > >> >>       but if null we use the cluster name.
> > > >> >>    - You've now commented out your whole executeQuery(Query)
> > method.
> > > >> Might
> > > >> >>    be on purpose, but I guess you had some nice optimizations
for
> > > >> special
> > > >> >>    cases in there. I hope we can restore that.
> > > >> >>
> > > >> >> What is your apache username? When we have that, I propose
that
> we
> > > take
> > > >> >> your contribution for a VOTE and then hopefully integrate
it into
> > the
> > > >> main
> > > >> >> codebase of MM. The corrections above can be made either
before
> or
> > > after
> > > >> >> IMO.
> > > >> >>
> > > >> >> Best regards,
> > > >> >> Kasper
> > > >> >>
> > > >> >>
> > > >> >> 2014-09-16 9:28 GMT+02:00 Alberto Rodriguez <
> > arodriguez@stratio.com
> > > >:
> > > >> >>
> > > >> >> > Hi all,
> > > >> >> >
> > > >> >> > I have been working on the ES module. As Kasper suggested
I
> have
> > > >> >> overriden
> > > >> >> > the materializeMainSchemaTable to get a first functional
> > version. I
> > > >> have
> > > >> >> > added quite a few tests to check that the module is
working
> fine
> > > for
> > > >> >> > *SELECT* operations. I have also implemented the
> > executeCountQuery
> > > >> >> method.
> > > >> >> >
> > > >> >> > I forked the project from the *master branch, *you can
check
> out
> > > the
> > > >> >> > changes here: incubator-metamodel fork with ES module
> > > >> >> > <https://github.com/albertostratio/incubator-metamodel>
> > > >> >> >
> > > >> >> > I have also been informed by the ASF that my ICLA has
been
> filed
> > in
> > > >> their
> > > >> >> > records.
> > > >> >> >
> > > >> >> > Kind regards,
> > > >> >> >
> > > >> >> >
> > > >> >> > Alberto Rodríguez
> > > >> >> >
> > > >> >> >
> > > >> >> > <http://www.stratio.com/>
> > > >> >> > Avenida de Europa, 26. Ática 5. 3ª Planta
> > > >> >> > 28224 Pozuelo de Alarcón, Madrid
> > > >> >> > Tel: +34 91 352 59 42 // *@stratiobd <
> > > https://twitter.com/StratioBD>*
> > > >> >> >
> > > >> >>
> > > >>
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message