Fixing es dependencies for #48 #74
Conversation
| @@ -34,7 +34,7 @@ Gem::Specification.new do |s| | |||
| s.version = SensuPluginsElasticsearch::Version::VER_STRING | |||
|
|
|||
| s.add_runtime_dependency 'rest-client', '1.8.0' | |||
| s.add_runtime_dependency 'elasticsearch', '~> 1.0.14' | |||
| s.add_runtime_dependency 'elasticsearch', '>= 1.0.10' | |||
There was a problem hiding this comment.
This will pull in the latest version. I suggest we cap it at a major version so we don't get burned later. Also, do we know for sure the 5.x gem will work fine against ES 2.x?
There was a problem hiding this comment.
@eheydrick I would imagine there are some incompatibilities, they are major versions which implies breaking changes. Unless we rely on pure rest or have 2 separate gems (ew) I see no way of guaranteeing and allowing it to span multiple major versions...
There was a problem hiding this comment.
I'm not seeing a clean way either.
As for the original issue in #48, I suspect it's actually the way the check is being invoked or an environmental issue. I have this exact same version installed and it works fine for me.
@kaushiksriram100 can you check out my comment at #48 (comment).
There was a problem hiding this comment.
FWIW I get the following
/opt/sensu/embedded/lib/ruby/gems/2.4.0/gems/elasticsearch-transport-1.0.18/lib/elasticsearch/transport/transport/base.rb:52: warning: constant ::Fixnum is deprecated
/opt/sensu/embedded/lib/ruby/gems/2.4.0/gems/elasticsearch-transport-1.0.18/lib/elasticsearch/transport/transport/base.rb:54: warning: constant ::Fixnum is deprecated
ESQueryCount CRITICAL: Query count (2) was above critical threshold.
There was a problem hiding this comment.
ah yes we need to update most of the gems to support ruby 2.4
There was a problem hiding this comment.
I have opened cdunn/aws-es-transport#5 to allow us to specify greater than es 1.x after this merges and releases we can bump it here...
|
|
||
| ### [1.3.1] - 2017-05-10 | ||
| ###Bug Fix | ||
| - added ES version dependencies in gemspec |
There was a problem hiding this comment.
Changelog additions should go at the top under Unreleased. No need to put the version and date as the maintainers will bump that at release time.
| @@ -2,7 +2,7 @@ module SensuPluginsElasticsearch | |||
| module Version | |||
| MAJOR = 1 | |||
| MINOR = 3 | |||
| PATCH = 0 | |||
| PATCH = 1 | |||
There was a problem hiding this comment.
No need to bump version. We'll handle that when it's time to release.
|
@kaushiksriram100 have you had a chance to see if #74 (comment) might be the case? |
|
@kaushiksriram100 just checking if you can validate if erics theory is right. |
|
closing due to inactivity, this will be revamped soon with lots of new versions and will be dropping stuff in some upcoming major releases. You can read about some of it here: https://blog.sensuapp.org/community-maintenance-mode-d109e75bcbc4 |
Pull Request Checklist
Is this in reference to an existing issue?
YES - #48
General
Update Changelog following the conventions laid out on Keep A Changelog
Update README with any necessary configuration snippets
Binstubs are created if needed
RuboCop passes
Existing tests pass
Purpose
Fix ES version compatability.
Known Compatablity Issues