Skip to content

Consolidate script parsing from object (7.x) #59509

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

Merged

Conversation

javanna
Copy link
Member

@javanna javanna commented Jul 14, 2020

This is the same as #59507 but without rejecting config that has unsupported fields, because that is breaking change that we don't want to make in 7.x

javanna added 2 commits July 14, 2020 11:00
The update by query action parses a script from an object (map or string). We will need to do the same for runtime fields as they are parsed as part of mappings (elastic#59391).

This commit moves the existing parsing of a script from an object from RestUpdateByQueryAction to the Script class. It also adds tests and adjusts some error messages that are incorrect. Also, options were not parsed before and they are now. And unsupported fields are no longer silently ignored.
@javanna javanna added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.9.0 labels Jul 14, 2020
@javanna javanna requested review from nik9000 and jdconrad July 14, 2020 09:09
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Scripting)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jul 14, 2020
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Nice!

@javanna
Copy link
Member Author

javanna commented Jul 14, 2020

I added a deprecation warning for unsupported fields so users are warned that this will cause an error in 8.0.

@nik9000
Copy link
Member

nik9000 commented Jul 14, 2020

I added a deprecation warning for unsupported fields so users are warned that this will cause an error in 8.0.

That seems like a good choice, yeah.

@javanna javanna merged commit af2f85b into elastic:7.x Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team v7.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants