metamodel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alberto Rodriguez <ardl...@gmail.com>
Subject Re: Elastic Search module - Progress update
Date Wed, 17 Sep 2014 21:51:14 GMT
Hi Kasper,

all your suggestions/changes have been done.

I am also happy to be involved in such an interesting project.

Kind regards,

Alberto

2014-09-17 21:21 GMT+02:00 Kasper Sørensen <i.am.kasper.sorensen@gmail.com>:

> 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