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

Accept more ingest simulate params as ints or strings #66197

Merged
merged 3 commits into from
Jan 11, 2021

Conversation

gaobinlong
Copy link
Contributor

Relates to #65992.

Seem to #23823.

_version, _if_seq_no and _if_primary_term params in the simulate pipeline API cannot be parsed correctly when the value is integer orstring type.

This PR fix the bug and modify the test method to test it.

@tvernum tvernum added the :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP label Dec 14, 2020
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Dec 14, 2020
@elasticmachine
Copy link
Collaborator

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

@tvernum
Copy link
Contributor

tvernum commented Dec 14, 2020

@elasticmachine test this please

@probakowski probakowski self-requested a review December 17, 2020 16:03
@probakowski
Copy link
Contributor

@elasticmachine ok to test

@probakowski
Copy link
Contributor

@elasticmachine update branch

Copy link
Contributor

@probakowski probakowski left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @gaobinlong for fixing this!
I'll get this merged and backported.

@probakowski probakowski merged commit f318f79 into elastic:master Jan 11, 2021
probakowski pushed a commit to probakowski/elasticsearch that referenced this pull request Jan 11, 2021
_version, _if_seq_no and _if_primary_term params in the simulate pipeline API cannot be parsed correctly when the value is integer orstring type.

This PR fix the bug and modify the test method to test it.
# Conflicts:
#	server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java
@probakowski
Copy link
Contributor

I found a problem during backport version, _if_seq_no and _if_primary_term are longs but currently simulate will fail if you provide number bigger than max integer. I have fix for that and I'll open PR soon and only merge it to 7.x after that (that mean it'll probably land in 7.13)

probakowski added a commit that referenced this pull request Feb 19, 2021
After #66197 we've lost ability to use longs for version, if_seq_no and if_primary_term (only ints would work).
That slipped through because 1 test went missing after refactoring in #50131.
This change fixes that with new ConfigurationUtils.readOptionalStringOrLongProperty method and and brings back the test.
probakowski added a commit to probakowski/elasticsearch that referenced this pull request Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement Team:Data Management Meta label for data/management team v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants