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

Deprecate X-Pack centric rollup endpoints #35962

Merged
merged 4 commits into from
Nov 28, 2018

Conversation

jasontedor
Copy link
Member

This commit is part of our plan to deprecate and ultimately remove the use of _xpack in the REST APIs.

Relates #35958

This commit is part of our plan to deprecate and ultimately remove the
use of _xpack in the REST APIs.
@jasontedor jasontedor added >deprecation v7.0.0 :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data labels Nov 27, 2018
@elasticmachine
Copy link
Collaborator

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);
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed 146acdd.

Copy link
Contributor

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");
Copy link
Contributor

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.

Copy link
Member Author

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!

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed 463198a.

@polyfractal
Copy link
Contributor

Left one more comment about the full cluster restart test, otherwise LGTM!

@jasontedor jasontedor merged commit b84f1f6 into elastic:master Nov 28, 2018
@jasontedor jasontedor deleted the xpack-rollup branch November 28, 2018 01:34
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Nov 28, 2018
* master:
  Deprecate X-Pack centric rollup endpoints (elastic#35962)
polyfractal added a commit that referenced this pull request Nov 29, 2018
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>deprecation :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants