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.

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

Broken Builds after 0.0.20 #48

Closed
driverpt opened this issue May 11, 2016 · 18 comments
Closed

Broken Builds after 0.0.20 #48

driverpt opened this issue May 11, 2016 · 18 comments
Labels

Comments

@driverpt
Copy link

As title says:

Using Accessor#strict_set for specs
NameError: undefined method 'set' for class 'LogStash::Event'
     alias_method at org/jruby/RubyModule.java:2333
            Event at /home/ubuntu/.rbenv/versions/jruby-1.7.22/lib/ruby/gems/shared/gems/logstash-devutils-0.0.20-java/lib/logstash/devutils/rspec/spec_helper.rb:38
           (root) at /home/ubuntu/.rbenv/versions/jruby-1.7.22/lib/ruby/gems/shared/gems/logstash-devutils-0.0.20-java/lib/logstash/devutils/rspec/spec_helper.rb:37
          require at org/jruby/RubyKernel.java:1040
           (root) at /home/ubuntu/workspace/<path_to_project>/spec/spec_helper.rb:1
          require at org/jruby/RubyKernel.java:1040
           (root) at /home/ubuntu/workspace/<path_to_project>/spec/spec_helper.rb:2
             load at org/jruby/RubyKernel.java:1059
@driverpt
Copy link
Author

I believe that this is being caused by the Plugin API 2.0.

Perhaps instead of tagging as 0.0.20, tag 0.1.0 instead ?

@jsvd
Copy link
Member

jsvd commented May 11, 2016

good catch. this gem should have an explicit dependency on logstash-core-plugin-api and maybe also logstash-core

@driverpt
Copy link
Author

Is this going to be reversed or fixed anytime soon ?

@jsvd
Copy link
Member

jsvd commented May 11, 2016

I suggest yanking this gem and releasing another with the constraint.

in what situation did this error occur? for now you can set in the Gemfile/gemspec the constraint to use logstash-devutils 0.0.19

@driverpt
Copy link
Author

driverpt commented May 11, 2016

@jsvd, it started to occur in our CI Server. I was able to trace the error to this situation.

The thing is it uses the latest minor-version s.add_development_dependency 'logstash-devutils', "~> 0.0.19" in the gemspec.

Thats why i suggested to re-tag this gem with 0.1.0 in the Plugin API v2.0 changes, so that the CI Servers do not start to break. See source

@colinsurprenant
Copy link
Contributor

Somewhat related to elastic/logstash/issues/5273 - we should also include this devutils issue.

As @jsvd suggested I think adding an explicit dependency on logstash-core-plugin-api in devutils should fix this. We should also remove versions constrains from plugins to devutils and let the resolution work through logstash-core-plugin-api. /cc @ph

Ultimately we should get rid of devutils altogether.

@ph
Copy link
Contributor

ph commented May 11, 2016

@driverpt @jsvd @colinsurprenant

I think the best path would be the following.

  • yank the 0.0.20 gems
  • Publish a new devutils to 1.0.0 (This might broke some plugins that depends on a specific version of devutils) I think we should go with a major bump, since its really breaking.

@colinsurprenant I don't think it can be handled by logstash-core-plugin-api, the devutils gem should be defined as a development dependency not a runtime one. This gems doesn't provide anything at runtime only when you run the test or you package the gem.

if we want logstash-core-plugin to control the version we would have to set it as a runtime dependency and I don't think we should do that.

If this gem is not set as a development dependency, I believe its a bug and we should fix that

@ph ph added the bug label May 11, 2016
@colinsurprenant
Copy link
Contributor

colinsurprenant commented May 11, 2016

@ph

  • yes, devutils is a development dependency
  • regardless if it is a development (or runtime) dependency, devutils itself could depend on a specific logstash-core-plugin-api version to make sure it is compatible with the correct logstash-core-* gems series - this will allow bundler to resolve the right version when building/testing

I am not sure why you say:

if we want logstash-core-plugin to control the version we would have to set it as a runtime dependency and I don't think we should do that.

Am I missing something?

@ph
Copy link
Contributor

ph commented May 11, 2016

@colinsurprenant take the following case.

Logstash-core-plugin-api has a development dependency on logstash-devutils
Logstash-filter-dns has a runtime dependency on logstash-code-plugin-api

bundle install will not pick up when its declared as a dev dependency.

But I have a better solution.

We can release logstash-devutils with a dependency on logstash-core-plugin-api ~> 2.0, this will fix the issue and its the easiest to do.

@ph
Copy link
Contributor

ph commented May 11, 2016

A quick search on all the plugins from logstash-plugins show that only 2 plugins require a specific version of devutils

/e/logstash-mass_effect git:master ❯❯❯ ag "logstash-devutils" | grep "~"                                                                                                                                                                                                                                         ⏎ ⬆ ✖ ✱ ◼
last_mass_pubilsh_went_wrong/tmp/logstash-input-beats/logstash-input-beats.gemspec:34:  s.add_development_dependency "logstash-devutils", "~> 0.0.18"
last_mass_pubilsh_went_wrong/tmp/logstash-input-file/logstash-input-file.gemspec:31:  s.add_development_dependency 'logstash-devutils', ['~> 0.0.18']
~/e/logstash-mass_effect git:master ❯❯❯

@colinsurprenant
Copy link
Contributor

colinsurprenant commented May 11, 2016

We can release logstash-devutils with a dependency on logstash-core-plugin-api ~> 2.0, this will fix the issue and its the easiest to do.

Yes, this is exactly what I was saying (twice) ^^ : add specific logstash-core-plugin-api version as a dependency to logstash-devutils

@ph
Copy link
Contributor

ph commented May 11, 2016

@colinsurprenant lets do that and update the 2 plugins that required relaxed dependency in their 2.0 master branch

ph added a commit to ph/logstash-devutils that referenced this issue May 11, 2016
Make sure we depends on logtash-core-api when the event api changes.

Fixes elastic#48
@colinsurprenant
Copy link
Contributor

@ph just to make sure we are ont he same page :P you mean to remove the version constraint on devutils in input-beats and input-file ?

ph added a commit to ph/logstash-input-beats that referenced this issue May 11, 2016
The right version for logstash-devutils will be selected on the
logstash-core-plugin-api constraints

Related to elastic/logstash-devutils#48
ph added a commit to ph/logstash-input-beats that referenced this issue May 11, 2016
The right version for logstash-devutils will be selected on the
logstash-core-plugin-api constraints

Related to elastic/logstash-devutils#48
@ph
Copy link
Contributor

ph commented May 11, 2016

@colinsurprenant exact

ph added a commit to ph/logstash-input-file that referenced this issue May 11, 2016
The right version for logstash-devutils will be selected on the
logstash-core-plugin-api constraints

Related to elastic/logstash-devutils#48
@ph
Copy link
Contributor

ph commented May 11, 2016

@driverpt I have yanked the .20 gems, so you can remove the constraints.

@colinsurprenant
Copy link
Contributor

thanks @ph !

@driverpt
Copy link
Author

@ph @colinsurprenant , tks for all your support.

If you allow me, i have a small suggestion, to use Semantic Versioning (semver.org) to prevent these issues from happening.

@ph
Copy link
Contributor

ph commented May 11, 2016

@driverpt We try to respect it, this was an honest mistake sorry for the inconvenience.

elasticsearch-bot pushed a commit to logstash-plugins/logstash-input-file that referenced this issue May 11, 2016
The right version for logstash-devutils will be selected on the
logstash-core-plugin-api constraints

Related to elastic/logstash-devutils#48

Fixes #126
elasticsearch-bot pushed a commit to logstash-plugins/logstash-input-beats that referenced this issue May 11, 2016
The right version for logstash-devutils will be selected on the
logstash-core-plugin-api constraints

Related to elastic/logstash-devutils#48

Fixes #86
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants