Skip to content

Accept more ingest simulate params as ints or strings#66197

Merged
probakowski merged 3 commits intoelastic:masterfrom
gaobinlong:fix_simulate_params
Jan 11, 2021
Merged

Accept more ingest simulate params as ints or strings#66197
probakowski merged 3 commits intoelastic:masterfrom
gaobinlong:fix_simulate_params

Conversation

@gaobinlong
Copy link
Copy Markdown
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 :Distributed/Ingest Node Execution or management of Ingest Pipelines label Dec 14, 2020
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Dec 14, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@tvernum
Copy link
Copy Markdown
Contributor

tvernum commented Dec 14, 2020

@elasticmachine test this please

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

@elasticmachine ok to test

@probakowski
Copy link
Copy Markdown
Contributor

@elasticmachine update branch

Copy link
Copy Markdown
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
Copy Markdown
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

:Distributed/Ingest Node Execution or management of Ingest Pipelines >enhancement Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v7.13.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants