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

[WIP] refactor Ruby Event getter and setter #5170

Merged
merged 1 commit into from
May 6, 2016

Conversation

colinsurprenant
Copy link
Contributor

@colinsurprenant colinsurprenant commented Apr 21, 2016

This is a WIP for the Event getter and setter refactor.

Per #5141 and #5140

This code is pushed in feature/event_interface shared branch.

For this to work the plugins using a local :path in the included modified Gemfile need to be tweeked to use the new interface.

  • Modify the Ruby Event interface
  • Pass core specs using Ruby Event
  • Modify Java Event interface
  • Pass core specs using Java Event
  • bump logstash-core-plugin-api for new Event interface
  • publish modified plugins required for core specs using the new logstash-core-plugin-api version

gem "tins", "1.6", :group => :development
gem "rspec", "~> 3.1.0", :group => :development
gem "logstash-devutils", "~> 0.0.15", :group => :development
gem "logstash-devutils", ">= 0", :path => "/Users/colin/dev/src/elasticsearch/logstash-devutils"
Copy link
Contributor

Choose a reason for hiding this comment

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

User specific path? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is not a mergeable WIP and I added in the description that these must be addressed...

@colinsurprenant
Copy link
Contributor Author

@ph @suyograo we need to discuss the strategy for bumping logstash-core-plugin-api + branching so we can start updating the plugins ...

@ph
Copy link
Contributor

ph commented Apr 23, 2016

@colinsurprenant I think the following strategy will work:

  • create a logstash-core-plugin-api 2.0.0
  • make a branch on every plugin and update them to use this gem.

I guess we need to bump major version of the plugin too?

@suyograo
Copy link
Contributor

+1 to what @ph said. Also update the gemspec for all plugins

@ph
Copy link
Contributor

ph commented Apr 23, 2016

@suyograo @colinsurprenant we should also be able to release the new plugins with their major version and test them. People wont be able to install them in version prior to < 5.0.

@colinsurprenant
Copy link
Contributor Author

+1

@ph
Copy link
Contributor

ph commented May 5, 2016

Test are passing, all the updated plugins are also passing.

LGTM.

@colinsurprenant
Copy link
Contributor Author

ok, so will push commit to remove branches plugins gem, then we can merge in master

@colinsurprenant
Copy link
Contributor Author

rebasing and merging.

refactor wip gemfiles

refactor Java Event getter and setter

bump plugin-api to 2.0

use plugin-api 2.0

switch to core-event-java

include logstash-core-event-java.jar jar file so that gem dependency using the source tree work

updated core plugins to core-api 2.0

added grok for refactor branch

fix rebased specs

remove temp plugins github paths

remove commented out tmp alias_method
@colinsurprenant colinsurprenant force-pushed the feature/event_interface branch from a94fb4f to ee2c8af Compare May 6, 2016 17:33
@colinsurprenant colinsurprenant merged commit ee2c8af into master May 6, 2016
@colinsurprenant
Copy link
Contributor Author

merged into master and 5.0

@suyograo suyograo mentioned this pull request May 13, 2016
@suyograo suyograo deleted the feature/event_interface branch May 19, 2016 19:10
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.

4 participants