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

Ensure target option is defined in Lookups when ECS is enabled #71

Conversation

andsel
Copy link
Contributor

@andsel andsel commented May 24, 2021

This PR add the validation of target sub-option in the blocks of local_lookups for logstash-filter-jdbc_static.

The target attribute already existed, in this case it's only added a check that it's valued in case ECS is enabled.

Related to PR #69
Related to issue #19

@andsel andsel force-pushed the feature/enable_ecs_for_jdbc_filter_static branch from bb023d0 to 70a9004 Compare May 25, 2021 12:25
@andsel andsel marked this pull request as ready for review May 25, 2021 12:34
@elasticsearch-bot elasticsearch-bot self-assigned this May 25, 2021
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

looking sharp, one slight change related to not relying on the i-var

lib/logstash/filters/jdbc_static.rb Outdated Show resolved Hide resolved
Instead of direct access to  @ecs_compatibility it uses the accessor method

Co-authored-by: Karol Bucek <kares@users.noreply.github.com>
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

LGTM

@andsel andsel mentioned this pull request May 26, 2021
@andsel andsel merged commit 7296094 into logstash-plugins:master May 26, 2021
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