elastic / logstash-devutils

An assortment of tooling/libraries to make Logstash core and plugin development and releasing a bit easier.
Apache License 2.0
17 stars 29 forks source link

Fix: unwrap output and refactor test sink #82

Closed kares closed 4 years ago

kares commented 4 years ago

the old way wasn't reliable in case of pipeline failures

this is now expected to be more robust at a cost of relying on some of the native internals (which are available since LS 6.3 - our baseline)

discovered at https://github.com/logstash-plugins/logstash-input-elasticsearch/pull/118#issuecomment-595434716 ... here's a failing build these are expected to resolve

colinsurprenant commented 4 years ago

LS 5 is still officially supported but I don't think we build or test against it. Do we know if tests are done against LS < 6.3 ? As I understand it, this patch would break on any version < 6.3 ?

kares commented 4 years ago

thank you for looking into this.

As I understand it, this patch would break on any version < 6.3 ?

yes LS 5.x would break but out 6.3 base-line (as this is devutils 2.0) is established already. (if a gem specifies add_development_dependency 'logstash-devutils' wout a version limiter it installs devutils 1.3 on LS 5 and 2.0 on anything >= 6.3 - we know this is working fine and plugins are updated to deal with 2.0's breaking changes)

the 6.3 note might have been confusing - more of a "note to self" that the internals we expect for the unwrapping :

  org.logstash.config.ir.compiler.OutputStrategyExt::SimpleAbstractOutputStrategyExt.class_eval do
    field_reader :output # available since LS 6.3
  end

.. are available since LS 6.3 (nut not anything < 6.3)