-
Notifications
You must be signed in to change notification settings - Fork 424
Upgrade of Elastic Search Client #571
base: master
Are you sure you want to change the base?
Conversation
Issue filed in JIRA: USERGRID-1344 |
Thanks for the PR. There is a large amount of formatting changes in the PR that makes it difficult to see what has changed. Please submit a clean PR that contains only the changes you wish to make and makes no formatting changes. Also, new code should follow the existing code style of Usergrid which uses spaces for indentation and no tabs. |
@snoopdave , As requested, removed the code formatting. Added just the updated code |
Thanks Vineet, this PR looks much better. A better description of this PR would be "Update the Chop testing framework from ElasticSearch 1.4.x/1.7.x to ElasticSearch 5.4.x" because this PR does not upgrade Usergrid itself to the new ElasticSearch -- just the Chop testing framework. (I wonder how much work it would be to upgrade Usegrid itself to ES 5.4.x) I think we should accept this PR. Any thoughts @michaelarusso and @mikedunker-apigee? |
I'm not as familiar with the Chop testing framework. However, I can't see how it's beneficial to add 5.4.x Elasticsearch dependencies to the testing framework when Usergrid itself is not updated to support anything newer than 1.x of Elasticsearch. In terms of effort to add support for later versions of Elasticsearch, I imagine this to be a big effort. There will be breaking changes, maybe many, between 1.x and 5.x versions of Elasticsearch. The approach has to be thought through very carefully, otherwise existing users of Usergrid will be left in an awkward situation when trying to upgrade. New users might be fine, but the breaking changes in the cluster for existing users may just support a major version bump in Usergrid itself. |
Since the Usergrid project does not use Chop, changes to Chop don't impact Usergrid -- Chop is a separate project that probably shouldn't even be part of the Usergrid codebase since it results in confusion. If Vineet is using Chop separately from Usergrid, perhaps these changes make sense to him -- and maybe we should accept them. If the intention of this PR was to upgrade Usergrid to ES 5.x, then that is not what it does -- and we should not accept the changes. |
@snoopdave Thanks for the clarification. Given that Chop is separate, do you think it makes sense to create another repo, usergrid-chop, and move the code over post-merge? |
@snoopdave @michaelarusso I would like to know if chop is the only component which uses Elastic Search and if elastic search is even one of the core components of User-grid or not? I had made the changes because while installing User-Grid, I used the latest Elastic Search version and got client issues in the logs. I updated the Client and tested and it works perfectly fine now. Therefore for User Grid to work with latest version of Elastic Search the changes are needed. Arranging Chop in a separate repository is another call. Please suggest |
Hi @snoopdave, is the pull request being taken up..? |
This pull request does not upgrade Usergrid to use ES 5.4.x, it only upgrades the completely separate Chop testing framework that is not part of Usergrid and is not used by Usergrid -- so I don't think we need it. If you want to upgrade Usergrid, you need to change the stack/corepersistence/queryindex module, that is how Usergrid interfaces with ES. |
Hi @snoopdave, Would like to confirm, if we can deploy User grid current version with Elastic Search 5.4.x and run smoothly. Please confirm |
I cannot confirm because Ihave not attempted to run with ES 5.4.x.
Dave
…On Tue, Jun 27, 2017 at 6:59 AM Vineet Verma ***@***.***> wrote:
Hi @snoopdave <https://github.com/snoopdave>,
Would like to confirm, if we can deploy User grid current version with
Elastic Search 5.4.x and run smoothly.
Please confirm
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#571 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABBt1e11J1PvbOhG-BzCMmHGFQLt3cyeks5sIOCPgaJpZM4NtBpY>
.
|
Hi @snoopdave i tried that and faced a lot of issues... Thus my reason to create this pull request |
Hi @snoopdave any update? |
Hi @snoopdave, Please confirm if the PR is going to be merged..?? |
I do not intended to merge this code. I don't think it makes sense to upgrade the Chop testing framework to ES 5 when Usergrid itself only supports ES 1. |
Upgrade from:
To