search

petalmd / armor

Apache License 2.0
15 stars 8 forks source link

Elasticsearch 2.1 #15

Closed ersushantsood closed 8 years ago

ersushantsood commented 8 years ago

Hi @jmaitrehenry I have modified the changes tested and verified with elasticsearch 2.1 , can you please verify them and then we can merge these changes to master .

jmaitrehenry commented 8 years ago

Hi @ersushantsood, We have 2 failing tests, once those will be fixed, I will merge without problem. Thanks for your works!

ersushantsood commented 8 years ago

Hi jmaitrehenry , I have a suggestion can we keep different branches for different Elasticsearch versions as there is no backward compatibility of the plugin with older versions

jmaitrehenry commented 8 years ago

I think it’s a good idea to keep a branch for ES1.7 but we can keep only one branch for 2.x? And the latest elasticsearch version should be available in master I think. What do you think?

On Apr 22, 2016, at 11:50 AM, Sushant Sood notifications@github.com wrote:

Hi jmaitrehenry , I have a suggestion can we keep different branches for different Elasticsearch versions as there is no backward compatibility of the plugin with older versions

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/petalmd/armor/pull/15#issuecomment-213485326

ersushantsood commented 8 years ago

Yap that would be ideal .We need to start working toward looking at supporting 2.2 and 2.3 versions also.

jmaitrehenry commented 8 years ago

@jehuty0shift & @ersushantsood I just rebase with master. If the next build is green, I will merge into master

@jehuty0shift do you want to check the rebase before the final merge?

jehuty0shift commented 8 years ago

Yep since i introduced these changes. I can fix the merge if you want. The idea behind this option was to make it possible to allow only the monitor actions that Kibana needs. Since Kibana needs more than monitors action now, i could maybe rewrite it into allow_kibana_actions ? (that will allows all monitor and kibana actions needed by Kibana).

jmaitrehenry commented 8 years ago

Sound good for me.

On Jun 17, 2016, at 8:49 AM, jehuty0shift notifications@github.com wrote:

Yep since i introduced these changes. I can fix the merge if you want. The idea behind this option was to make it possible to allow only the monitor actions that Kibana needs. Since Kibana need more than monitors action now, i could maybe rewrite it into allow_kibana_actions ? (that will allows all monitor and kibana actions needed by Kibana).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/petalmd/armor/pull/15#issuecomment-226760314, or mute the thread https://github.com/notifications/unsubscribe/AAkOmvOzJ05RpzClMeIGlZ5yR7XJpIITks5qMpfggaJpZM4INamM.