Regarding the git workflow ... I hope we can eventually make the GitHub pull requests work integrated with our Apache Git server. I think Pull Requests are awesome to work with and makes life easy for everybody. Whether we integrate code is always going to be a VOTE and committer decision I guess. But we should probably consider this closer for some project by-laws so that things are transparent.
2014-09-24 10:03 GMT+02:00 Alberto Rodriguez <[email protected]>: > Sounds good! I will open JIRAs for things I think we can improve. > > Regarding the code changes... what is going to be the workflow? Have you > thought about following the gitflow approach? Something like this: git-flow > branching model <http://nvie.com/posts/a-successful-git-branching-model/> > > I think your point about a streaming DataSet is very interesting in fact we > have already done a pretty simple spike to find out how the memory > increases for the MongoDB MM module when querying big tables. Is there any > module with a "streaming" DataSet yet? > > Kind regards, > > Alberto > > 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-24 9:38 GMT+02:00 Kasper Sørensen <[email protected] > >: > > > OK, so I'll push that change to the master soon. > > > > I guess the next step is to start making JIRAs for stuff we want to > > improve. That could be to support insert/update/delete (ie. implement > > UpdateableDataContext interface) or it could be other improvements (for > > instance I noticed that the DataSet doesn't look to be streaming, so > large > > datasets would cause out of memory exceptions (I think, didn't look very > > closely into it yet)). > > > > You are more than welcome to start putting in those things into JIRA. > Then > > we have a nice process going where issues can be dealt with individually > > and we can track if there's more on our wish-list. > > > > Best regards, > > Kasper > > > > 2014-09-24 9:27 GMT+02:00 Alberto Rodriguez <[email protected]>: > > > > > Hi Kasper, > > > > > > I've been having a look at the changes and they all look fine with me. > I > > am > > > not really used to this kind of revisions...what should be the next > > steps? > > > > > > By the way, I did not have time to work on the insert, delete and > update > > > operations, do you think we should implement them? > > > > > > Kind regards, > > > > > > Alberto > > > > > > 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-21 22:55 GMT+02:00 Kasper Sørensen < > > [email protected] > > > >: > > > > > > > Here's my suggested changes to make it single-index based. I also > took > > > the > > > > liberty to clean up the code style a little to be in line with all > the > > > > other MM code. Hope that's OK. > > > > > > > > https://reviews.apache.org/r/25883/ > > > > > > > > 2014-09-21 22:28 GMT+02:00 Kasper Sørensen < > > > [email protected] > > > > >: > > > > > > > > > Yeah, let's make it single-schema oriented as a start. We might > later > > > add > > > > > multi-schema support to it, if we can also get around that in the > > > > > superclass. > > > > > > > > > > The JdbcDataContext does not inherit from > QueryPostprocessDataContext > > > at > > > > > all - it's a completely separate implementation that has all the > > schema > > > > > model handling and query execution based on JDBC and encoding the > > > queries > > > > > into SQL. That's of course by far more effecient, but also a lot > more > > > > work > > > > > than just using the QueryPostprocessDataContext's query handling > > > > mechanism. > > > > > > > > > > 2014-09-21 19:55 GMT+02:00 Alberto Rodriguez <[email protected]>: > > > > > > > > > >> Yes, we might force the user to pass in the index name on the > > > > DataContext > > > > >> creation. This way, if the user wants to point to another index he > > > > should > > > > >> create another DataContext which is not so bad IMHO. > > > > >> > > > > >> I am out of home this weekend and I can't have a look at MM > > repo...how > > > > do > > > > >> you handle this for JDBC? > > > > >> > > > > >> Kind Regards, > > > > >> > > > > >> Alberto > > > > >> El 21/09/2014 15:18, "Kasper Sørensen" < > > > [email protected]> > > > > >> escribió: > > > > >> > > > > >> > Hmm I imagine that we would not be happy with that solution in > the > > > > long > > > > >> run > > > > >> > and then would have to change it in the future anyway. And that > > kind > > > > of > > > > >> > change is a bit painful because it's only a runtime backwards > > > > >> > incompatibility so people will find out too late. > > > > >> > > > > > >> > We could also consider to make the implementation index > specific, > > > ie. > > > > >> only > > > > >> > for a single index. That would make a lot of things easy to > model > > > too. > > > > >> > Den 21/09/2014 12.29 skrev "Alberto Rodriguez" < > [email protected] > > >: > > > > >> > > > > > >> > > Hi Kasper, > > > > >> > > > > > > >> > > That's great news!! Thank you for merging the code! > > > > >> > > > > > > >> > > Regarding the issue you found out, as a simple workaround, > what > > if > > > > we > > > > >> > > prepend the index name to the "table" name,something like: > > > > >> > > indexname.indextable > > > > >> > > > > > > >> > > Kind regards, > > > > >> > > > > > > >> > > Alberto > > > > >> > > Hi Alberto, > > > > >> > > > > > > >> > > As you've probably seen, the code has now been merged with the > > > main > > > > >> > > MetaModel repo - congratulations and thank you a lot! > > > > >> > > > > > > >> > > After merging it I took it for another spin to really dive > deep > > > into > > > > >> some > > > > >> > > of the design choices made. I had an observation (and just > > > > committed a > > > > >> > > javadoc TODO comment about it) - the approach of mapping > indexes > > > and > > > > >> > > document types is potentially destructable in the sense that > if > > > you > > > > >> have > > > > >> > > multiple indexes with the same document types, those table > names > > > are > > > > >> > going > > > > >> > > to be in conflict. Everything is right now put under the same > > > > schema, > > > > >> > while > > > > >> > > I think actually the ideal approach would be to produce > multiple > > > > >> schemas, > > > > >> > > with their document types represented as tables. That would > > > reflect > > > > >> > better > > > > >> > > on the hierarchy that ES has. > > > > >> > > > > > > >> > > Obviously that's harder to do ... And unfortunately right now > > the > > > > >> > > QueryPostprocessDataContext class dictates that you have to > > > produce > > > > a > > > > >> > > single "main" schema. Which is kinda stupid and > circumstantial. > > > So I > > > > >> > > propose we see if we can fix that, and then make the ES module > > > > >> > multi-schema > > > > >> > > oriented. > > > > >> > > > > > > >> > > 2014-09-17 23:51 GMT+02:00 Alberto Rodriguez < > [email protected] > > >: > > > > >> > > > > > > >> > > > 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 < > > > > >> > > [email protected] > > > > >> > > > >: > > > > >> > > > > > > > >> > > > > 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 < > > > > >> [email protected] > > > > >> > >: > > > > >> > > > > > > > > >> > > > > > 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 < > > > > >> > > > > [email protected] > > > > >> > > > > > >: > > > > >> > > > > > > > > > >> > > > > > > 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" < > > > > >> > > [email protected] > > > > >> > > > >: > > > > >> > > > > > > > > > > >> > > > > > > > Thanks Alberto, this should help review the patch > > > > >> > > > > > > > > > > > >> > > > > > > > - Henry > > > > >> > > > > > > > > > > > >> > > > > > > > On Tue, Sep 16, 2014 at 1:44 PM, Alberto Rodriguez < > > > > >> > > > > [email protected]> > > > > >> > > > > > > > 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 < > > > > >> > > > [email protected] > > > > >> > > > > >: > > > > >> > > > > > > > > > > > > >> > > > > > > > >> 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 > > > > >> > > > > > > > >> <[email protected]> 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 < > > > > >> > > > > > > > >> [email protected]>: > > > > >> > > > > > > > >> > > > > > >> > > > > > > > >> >> 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 < > > > > >> > > > > > > [email protected] > > > > >> > > > > > > > >: > > > > >> > > > > > > > >> >> > > > > >> > > > > > > > >> >> > 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>* > > > > >> > > > > > > > >> >> > > > > > >> > > > > > > > >> >> > > > > >> > > > > > > > >> > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > > > >
