-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
backport 936faba6c43 to 6.x #35045
backport 936faba6c43 to 6.x #35045
Conversation
Pinging @elastic/es-core-infra |
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 left a few minor comments, looks good otherwise.
= new ConstructingObjectParser<>("delete_rollup_job_response", true, | ||
args -> new DeleteRollupJobResponse((boolean) args[0])); | ||
static { | ||
PARSER.declareBoolean(constructorArg(), new ParseField("acknowledged")); |
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.
nit: I changed this on master to get the parser from AcknowledgedResponse using a new generateParser
method that I introduced there on request of Baz. Maybe we could use the same here in the backport to make it match the version on master.
= new ConstructingObjectParser<>("put_rollup_job_response", true, args -> new PutRollupJobResponse((boolean) args[0])); | ||
static { | ||
PARSER.declareBoolean(constructorArg(), new ParseField("acknowledged")); | ||
} |
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.
same here: I'd like to keep the version using generateParser
since its the same as in master.
@@ -300,14 +300,19 @@ include::migration/get-assistance.asciidoc[] | |||
:doc-tests-file: {doc-tests}/RollupDocumentationIT.java | |||
|
|||
|
|||
:upid: {mainid}-rollup | |||
:doc-tests-file: {doc-tests}/RollupDocumentationIT.java |
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.
duplication from line 299/300
@cbuescher no rush, but I believe I have addressed your comments and this is ready for re-review. thank you! |
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.
Many thanks, LGTM.
This PR backports delete job from
master
to6.x
branch.