Skip to content

Conversation

@original-brownbear
Copy link
Contributor

@original-brownbear original-brownbear commented May 18, 2018

  • PluginFactory ported 1:1 without behaviour changes
    • I know the duplicate method for Java and Ruby here suck, the good news is that the Ruby one's can go away once we delete the Ruby exec :)
  • Added Java wrapper for plugin lookup (since I couldn't port that (LogStash::Plugin and at least not yet) to Java unfortunately :()
  • This also introduces the ground work for the Java API and standing up Java plugins API wise, relevant work based on this will go to the Java feature branch once this is in master :)

Copy link
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

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

LGTM

@original-brownbear
Copy link
Contributor Author

@danhermann thanks!

@elasticsearch-bot
Copy link

Armin Braun merged this into the following branches!

Branch Commits
master 7a7b47f
6.x b4ea520

elasticsearch-bot pushed a commit that referenced this pull request May 18, 2018
@original-brownbear original-brownbear deleted the port-more-plugin-factory branch May 18, 2018 16:27
@webmat
Copy link
Contributor

webmat commented May 18, 2018

An example of the failure:

  2) outputs/loggly when initializing should register
     Failure/Error: expect { subject.register }.to_not raise_error
     
       expected no Exception, got #<NameError: uninitialized constant LogStash::PLUGIN_REGISTRY
       Did you mean?  LogStash::PluginMixins> with backtrace:
         # /home/travis/build/logstash/logstash-core/lib/logstash/plugin.rb:136:in `lookup'
         # /home/travis/build/logstash/logstash-core/lib/logstash/config/mixin.rb:413:in `validate_value'
         # /home/travis/build/logstash/logstash-core/lib/logstash/config/mixin.rb:332:in `process_parameter_value'
         # /home/travis/build/logstash/logstash-core/lib/logstash/config/mixin.rb:351:in `block in validate_check_parameter_values'
         # /home/travis/build/logstash/logstash-core/lib/logstash/config/mixin.rb:345:in `block in validate_check_parameter_values'
         # /home/travis/build/logstash/logstash-core/lib/logstash/config/mixin.rb:344:in `validate_check_parameter_values'
         # /home/travis/build/logstash/logstash-core/lib/logstash/config/mixin.rb:234:in `validate'
         # /home/travis/build/logstash/logstash-core/lib/logstash/config/mixin.rb:85:in `config_init'
         # /home/travis/build/logstash/logstash-core/lib/logstash/outputs/base.rb:60:in `initialize'
         # ./spec/outputs/loggly_spec.rb:27:in `block in subject'
         # ./spec/outputs/loggly_spec.rb:30:in `block in (root)'
         # ./spec/outputs/loggly_spec.rb:30:in `block in (root)'
         # /home/travis/.rvm/gems/jruby-9.1.13.0/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block in (root)'
     # ./spec/outputs/loggly_spec.rb:30:in `block in (root)'
     # /home/travis/.rvm/gems/jruby-9.1.13.0/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block in (root)'

@original-brownbear
Copy link
Contributor Author

@webmat ok it's pretty obvious that me removing a cyclical include here caused this. I can fix this shortly (tomorrow).

@webmat
Copy link
Contributor

webmat commented May 18, 2018

Not sure how far away in terms of timezones we are. But first thing next week works as well ;-) Don't work on a Saturday just because of me :-)

@andrewvc
Copy link
Contributor

@original-brownbear I'm going to revert this for now. We also need to fix: https://github.com/elastic/logstash/blob/master/logstash-core/lib/logstash/plugin.rb#L135 to use the new classes.

@original-brownbear
Copy link
Contributor Author

@andrewvc just copy registry into plugin :) => fixed. The problem here is the stupid cyclic dependency between plugin and registry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants