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

Feat: introduce a target check helper #6

Merged
merged 10 commits into from
Jun 7, 2021

Conversation

kares
Copy link
Contributor

@kares kares commented May 25, 2021

for re-use across plugins - unifying the message, usage:

class LogStash::Filters::Json < LogStash::Filters::Base

  include LogStash::PluginMixins::ECSCompatibilitySupport
  include LogStash::PluginMixins::ECSCompatibilitySupport::TargetCheck

  extend LogStash::PluginMixins::ValidatorSupport::FieldReferenceValidationAdapter
  
  config :target, :validate => :field_reference
  # ...
  
  def register
    check_target_set_in_ecs_mode
    # ...
  end
  
end

kares added 2 commits May 25, 2021 13:05
for re-use across plugins - unifying the message
@kares kares requested a review from yaauie May 25, 2021 11:34
Copy link
Contributor

@yaauie yaauie left a comment

Choose a reason for hiding this comment

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

In general, this certainly is on track.

Thinking about this purely from a usage perspective, it would be very nice if a developer could include a single module into their plugin and not have to do anything else (such as overriding Plugin#register or modifying their existing override). That way we could validate the behaviour extensively here in this support gem, and consumers would simply need to validate the presence of this support gem in their plugin ancestors without also re-validating its behaviour at a unit level.

I've included some additional thoughts and pseudo-code in the diff.

kares and others added 2 commits May 31, 2021 11:15
…heck_spec.rb

Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>
regardless of ECS compatibility setting
@kares kares requested a review from yaauie May 31, 2021 09:29
kares and others added 5 commits June 7, 2021 09:48
…heck_spec.rb

Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>
…heck_spec.rb

Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>
…heck_spec.rb

Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>
…eck.rb

Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>
…eck.rb

Co-authored-by: Ry Biesemeyer <yaauie@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants