-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GeoIP clean up database after new download #12689
Conversation
PR is waiting for logstash-plugins/logstash-filter-geoip#176 to publish and allow Logstash to unblock test cases |
ca909d8
to
43ea2ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a little suggestion to improve readability, maybe.
LGTM
|
||
database_path | ||
return database_path if file_exist?(database_path) | ||
return database_path if database_path = get_file_path("GeoLite2-#{@database_type}.mmdb") and file_exist?(database_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand this if statement
checks for default database path, so be more specific could help readability:
return database_path if database_path = get_file_path("GeoLite2-#{@database_type}.mmdb") and file_exist?(database_path) | |
default_database_path = get_file_path("GeoLite2-#{@database_type}.mmdb") | |
return default_database_path if default_database_path and file_exist?(default_database_path) |
Maybe the not nil
check of default_database_path
could be embedded into file_exists?
This commit adds a clean-up step to remove .gz and .mmdb which is not in the metadata after pointing to the new GeoIP database Fixed: elastic#12560
This commit adds a clean-up step to remove .gz and .mmdb which is not in the metadata after pointing to the new GeoIP database Fixed: elastic#12560
This PR adds a clean-up step after a new GeoIP database download.
After pointing to new database, database_manager removes .gz and .mmdb which is not in the metadata.
Related issues
Logstash PR #12675
Meta #12560