Skip to content
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

Make GeoIpDownloaderIT.testInvalidTimestamp more robust #75330

Merged
merged 3 commits into from
Jul 14, 2021

Conversation

probakowski
Copy link
Contributor

@probakowski probakowski commented Jul 14, 2021

This change adds additional assertion in GeoIpDownloaderIT.testInvalidTimestamp which makes sure that validity checks work both ways (so going out of validity and back) and it should fix race in cleanUp method leading to occasional failures.

Closes #75221
Closes #74358

@probakowski probakowski added >test Issues or PRs that are addressing/adding tests :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v8.0.0 auto-backport Automatically create backport pull requests when merged v7.14.1 v7.15.0 labels Jul 14, 2021
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Jul 14, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, I left one super minor comment

client().admin().cluster()
.prepareUpdateSettings()
.setPersistentSettings(Settings.builder()
.put("ingest.geoip.database_validity", (String) null))
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit:

Suggested change
.put("ingest.geoip.database_validity", (String) null))
.putNull("ingest.geoip.database_validity")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I haven't seen that method before, I've update it in other places in this test as well

@probakowski
Copy link
Contributor Author

@elasticmachine update branch

@probakowski probakowski merged commit 0f2e221 into elastic:master Jul 14, 2021
@probakowski probakowski deleted the geoip-timestamp-test branch July 14, 2021 23:27
elasticsearchmachine pushed a commit to elasticsearchmachine/elasticsearch that referenced this pull request Jul 14, 2021
This change adds additional assertion in GeoIpDownloaderIT.testInvalidTimestamp which makes sure that validity checks work both ways (so going out of validity and back) and it should fix race in cleanUp method leading to occasional failures.

Closes elastic#75221
Closes elastic#74358
elasticsearchmachine pushed a commit to elasticsearchmachine/elasticsearch that referenced this pull request Jul 14, 2021
This change adds additional assertion in GeoIpDownloaderIT.testInvalidTimestamp which makes sure that validity checks work both ways (so going out of validity and back) and it should fix race in cleanUp method leading to occasional failures.

Closes elastic#75221
Closes elastic#74358
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
7.14
7.x

elasticsearchmachine added a commit that referenced this pull request Jul 15, 2021
… (#75353)

* Make GeoIpDownloaderIT.testInvalidTimestamp more robust (#75330)

This change adds additional assertion in GeoIpDownloaderIT.testInvalidTimestamp which makes sure that validity checks work both ways (so going out of validity and back) and it should fix race in cleanUp method leading to occasional failures.

Closes #75221
Closes #74358

* compilation fix

Co-authored-by: Przemko Robakowski <[email protected]>
elasticsearchmachine added a commit that referenced this pull request Jul 15, 2021
#75354)

* Make GeoIpDownloaderIT.testInvalidTimestamp more robust (#75330)

This change adds additional assertion in GeoIpDownloaderIT.testInvalidTimestamp which makes sure that validity checks work both ways (so going out of validity and back) and it should fix race in cleanUp method leading to occasional failures.

Closes #75221
Closes #74358

* compilation fix

Co-authored-by: Przemko Robakowski <[email protected]>
masseyke pushed a commit to masseyke/elasticsearch that referenced this pull request Jul 16, 2021
This change adds additional assertion in GeoIpDownloaderIT.testInvalidTimestamp which makes sure that validity checks work both ways (so going out of validity and back) and it should fix race in cleanUp method leading to occasional failures.

Closes elastic#75221
Closes elastic#74358
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v7.14.0 v7.15.0 v8.0.0-alpha1
Projects
None yet
5 participants