Skip to content

Conversation

@kares
Copy link

@kares kares commented Mar 28, 2022

actual option implementation provided by mixin logstash-plugins/logstash-mixin-http_client#40

NOTE: feature is tested in http output, test here do not include any ssl_ related testing.

s.add_runtime_dependency 'logstash-core-plugin-api', '>= 1.60', '<= 2.99'
s.add_runtime_dependency 'logstash-mixin-ecs_compatibility_support', '~>1.2'
s.add_runtime_dependency "logstash-mixin-http_client", ">= 7.1.0", '< 9.0.0'
s.add_runtime_dependency "logstash-mixin-http_client", ">= 7.2.0", '< 9.0.0'
Copy link

Choose a reason for hiding this comment

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

The http_poller plugin doens't have the < 9.0.0 to you think we could remove it?

Copy link
Author

Choose a reason for hiding this comment

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

this is a common pattern ruby gems use to limit using a later version of the library,
you know there's a current 7.x versions that your code works with, 8.x will probably work as well but another major might be a bit too optimistic of an assumption - thus the upper bound limit.

Copy link

@andsel andsel left a comment

Choose a reason for hiding this comment

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

LGTM
Left just a note about the mixin upper bound limit, I was wonder if you think we could remove it, also http_poller input plugin doesn't have it.
However feel free to skip and merge this,

@kares kares self-assigned this Mar 28, 2022
@kares kares merged commit fd0dbee into logstash-plugins:main Mar 28, 2022
@kares kares mentioned this pull request Mar 28, 2022
40 tasks
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.

3 participants