use EULA GeoIP2 Database with auto-update in Elastic license #176
Conversation
|
|
||
| def load_database_manager? | ||
| begin | ||
| require_relative "#{LogStash::Environment::LOGSTASH_HOME}/x-pack/lib/filters/geoip/database_manager" |
There was a problem hiding this comment.
try to load the database manager from xpack
| require_relative "#{LogStash::Environment::LOGSTASH_HOME}/x-pack/lib/filters/geoip/database_manager" | ||
| true | ||
| rescue LoadError => e | ||
| @logger.info("DatabaseManager is not in classpath", :version => LOGSTASH_VERSION, :exception => e) |
There was a problem hiding this comment.
log in info level for old version user
|
This PR is pending on new docker image logstash-8.0.0-SNAPSHOT |
kaisecheng
force-pushed
the
geoip_database_xpack
branch
from
1c65082
to
4f8b64c
Compare
There was a problem hiding this comment.
@kaisecheng I'left a couple of comments, one minor method extraction and a doubt about versioned tests.
lib/logstash/filters/geoip.rb
Outdated
| raise "You must specify 'database => ...' in your geoip filter (I looked for '#{@database}')" | ||
| end | ||
| end | ||
| database_path = if load_database_manager? |
There was a problem hiding this comment.
I would extract this if in a separate method, for example select_database_path so the that it could also be separately tested and at the same time streamlines the actions of the register
| expect(::File.exist?(METADATA_PATH)).to be_falsey | ||
| end | ||
| end | ||
| end if MAJOR < 7 || (MAJOR == 7 && MINOR <= 12) |
There was a problem hiding this comment.
I've a doubt on this conditional tests. They decision of which is executed is based on the values present in https://github.com/logstash-plugins/.ci/blob/b20255c2da0c7fa82e9a247f6366e2da61b47e6c/travis/matrix.yml#L18.
Should we grant runs of these tests with Logstash 7.12 and Logstash 7.13+? In this way we grant no regressions in future releases of the plugin till the End-of-Life for 7.12, then we can drop those.
WDYT?
There was a problem hiding this comment.
As discussed, it is better to pin test against 7.11.0 to prevent losing track on < 7.12 in the future
kaisecheng
force-pushed
the
geoip_database_xpack
branch
from
53ea489
to
4a106a1
Compare
This PR is a split from #172
Additionally, the plugin resolves xpack database manager to run auto-update, otherwise, run the offline CC database