search

logstash-plugins / logstash-input-jdbc

Logstash Plugin for JDBC Inputs
Apache License 2.0
449 stars 187 forks source link

Jdk11 class loading #343

Closed guyboertje closed 5 years ago

guyboertje commented 5 years ago

This is the implementation of a wrapped class loader as suggested by Dan.

There is a small code re-org and due to a Jruby bug in 9.2.X, a monkey patch to tzinfo.

Gemspec and docs changes to follow.

colinsurprenant commented 5 years ago

Per my above code comments, I'd like for monkey patches to be documented, that is, if a method is redefined it should be documented as such; future developers will then directly know without having do dig in the original class to figure that out. Also, I'd like the inclusion of a monkey patch to be guarded by a version check; any future upgrade of JRuby will potentially conflicts with this monkey patch and we need an explicit way of forcing that verification at build time. The version check can simply fail the build if the version is different than the version upon which that monkey patch was built for.

guyboertje commented 5 years ago

It occurs to me that due to BWC the patch can only really be removed when the build matrix is has all versions using JRuby 9.2.8.0 and newer. The build failure I have now will act as a reminder to circle back to revisit this as and when the build matrix fails the 7.X and 8.X SNAPSHOT lines.

colinsurprenant commented 5 years ago

this will generate an exception at runtime wouldn't be better to crash in the tests/specs only?