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.

Sign up for GitHub

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

Jump to bottom

Deprecate X-Pack centric watcher endpoints #36218

Merged
merged 8 commits into from
Dec 8, 2018
Merged

Deprecate X-Pack centric watcher endpoints #36218

merged 8 commits into from
Dec 8, 2018

Conversation

hub-cap
Copy link
Contributor

@hub-cap hub-cap commented Dec 4, 2018

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

Relates #35958

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@jasontedor jasontedor mentioned this pull request Dec 4, 2018
Closed
11 tasks
jasontedor
jasontedor requested changes Dec 4, 2018
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

The endpoint can not be /watcher, that is not a reserved name. It needs to be /_watcher so that it can not possibly conflict with existing indices that users might have today.

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 4, 2018

oh my, thank you for that clarification @jasontedor

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 4, 2018

@jasontedor updated, as well as reverted any changes i made to the transport layer.

jasontedor
jasontedor requested changes Dec 4, 2018
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

I am missing where the existing endpoints are deprecated?

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 4, 2018

Because I am an idiot and I was trying to do this before finishing other work I should have finished first, and did not pay attention in the process... Let me properly do this and then Ill ping you. Im sorry for causing this many re-reviews on what should've been an easy PR.

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 4, 2018

@elasticmachine run the gradle build tests 1

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 4, 2018

I have to do the 6.x undeprecate first, and then I can rerun the tests here.

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 5, 2018

This depends on #36269 merging before the tests will go green.

jasontedor
jasontedor approved these changes Dec 7, 2018
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM now.

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 7, 2018

@elasticmachine run the gradle build tests 1

1 similar comment
@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 8, 2018

@elasticmachine run the gradle build tests 1

@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 8, 2018

@elasticmachine run the gradle build tests 2

1 similar comment
@hub-cap
Copy link
Contributor Author

hub-cap commented Dec 8, 2018

@elasticmachine run the gradle build tests 2

@hub-cap hub-cap merged commit b5b6e37 into elastic:master Dec 8, 2018
@lcawl lcawl mentioned this pull request Dec 10, 2018
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasontedor jasontedor jasontedor approved these changes

Assignees
No one assigned
Labels
:Data Management/Watcher >deprecation v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hub-cap @elasticmachine @jasontedor @colings86