Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing es dependencies for #48 #74

Closed
wants to merge 2 commits into from
Closed

Fixing es dependencies for #48 #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c397377
fixing elasticsearch gem dependencies. Issue 48
kaushik100 May 10, 2017
7e15adb
Issue #48
kaushik100 May 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,7 @@ This CHANGELOG follows the format listed at [Keep A Changelog](http://keepachang
[0.1.1]: https://github.com/sensu-plugins/sensu-plugins-elasticsearch/compare/0.1.0...0.1.1
[0.1.0]: https://github.com/sensu-plugins/sensu-plugins-elasticsearch/compare/0.0.2...0.1.0
[0.0.2]: https://github.com/sensu-plugins/sensu-plugins-elasticsearch/compare/0.0.1...0.0.2

### [1.3.1] - 2017-05-10
###Bug Fix
- added ES version dependencies in gemspec
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 changes: 1 addition & 1 deletion lib/sensu-plugins-elasticsearch/version.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module SensuPluginsElasticsearch
module Version
MAJOR = 1
MINOR = 3
PATCH = 0
PATCH = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to bump version. We'll handle that when it's time to release.


VER_STRING = [MAJOR, MINOR, PATCH].compact.join('.')
end
Expand Down
2 changes: 1 addition & 1 deletion sensu-plugins-elasticsearch.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@majormoses majormoses May 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link

@johntdyer johntdyer Jun 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eheydrick / @majormoses

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes we need to update most of the gems to support ruby 2.4

Copy link
Member

@majormoses majormoses Jun 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

s.add_runtime_dependency 'aws-es-transport', '~> 0.1'
s.add_runtime_dependency 'aws-sdk', ['>= 2.1.14', '< 2.5', '~> 2.1']
s.add_runtime_dependency 'sensu-plugin', '~> 1.2'
Expand Down