-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Deprecate X-Pack centric rollup endpoints #35962
Conversation
This commit is part of our plan to deprecate and ultimately remove the use of _xpack in the REST APIs.
Pinging @elastic/es-analytics-geo |
// TODO: remove deprecated endpoint in 8.0.0 | ||
controller.registerWithDeprecatedHandler( | ||
DELETE, "/_rollup/job/{id}", this, | ||
DELETE, Rollup.BASE_PATH + "job/{id}/", deprecationLogger); |
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 may be reading this wrong, but I think the deprecated path (Rollup.BASE_PATH + "job/{id}/"
) will resolve to /_rollup/job/{id}
because BASE_PATH
itself was changed above to no longer include _xpack
?
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.
Oops, that was an overzealous find/replace. That one should not have been changed.
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 pushed 146acdd.
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.
👍
@@ -261,7 +261,7 @@ public void testRollupAfterRestart() throws Exception { | |||
client().performRequest(bulkRequest); | |||
|
|||
// create the rollup job | |||
final Request createRollupJobRequest = new Request("PUT", "/_xpack/rollup/job/rollup-job-test"); | |||
final Request createRollupJobRequest = new Request("PUT", "/_rollup/job/rollup-job-test"); |
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.
Since this is a full cluster restart bwc test, I think we could go from 6.x to 7.0 right? Do we need to check if we're on 6.x so we can use the old endpoint?
BWC test always hurt my brain so maybe not.
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.
Yes, that would be right!
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 pushed 463198a.
Left one more comment about the full cluster restart test, otherwise LGTM! |
* master: Deprecate X-Pack centric rollup endpoints (elastic#35962)
This reverts commit b84f1f6.
)"" This reverts commit 61c2db5.
This commit is part of our plan to deprecate and ultimately remove the use of _xpack in the REST APIs.
Relates #35958