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

Use watch param instead of deprecated /watch/ prefix #41722

Merged
merged 3 commits into from
Feb 20, 2017
Merged

Use watch param instead of deprecated /watch/ prefix #41722

merged 3 commits into from
Feb 20, 2017

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Feb 19, 2017 •
edited
Loading

Switches clients to use watch param instead of /watch/ prefix

Clients now use the `?watch=true` parameter to make watch API calls, instead of the `/watch/` path prefix

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 19, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 19, 2017
@liggitt liggitt changed the title Switch from using the deprecated /watch/ prefix to using the watch param Use watch param instead of deprecated /watch/ prefix Feb 19, 2017
@liggitt liggitt added this to the v1.6 milestone Feb 19, 2017
@liggitt liggitt force-pushed the watch-prefix branch 2 times, most recently from 44bcdcd to 301b2c4 Compare February 19, 2017 20:11
@liggitt liggitt added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Feb 19, 2017
@liggitt liggitt force-pushed the watch-prefix branch 3 times, most recently from 135225f to dd338be Compare February 20, 2017 00:29
@smarterclayton
Copy link
Contributor

/approve

Lgtm and will mark when green

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 20, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

The following people have approved this PR: liggitt, smarterclayton

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@liggitt
Copy link
Member Author

liggitt commented Feb 20, 2017

@k8s-bot gci gce e2e test this

@smarterclayton
Copy link
Contributor

/lgtm

@k8s-ci-robot
Copy link
Contributor

@smarterclayton: you can't LGTM a PR unless you are an assignee.

In response to this comment:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@smarterclayton smarterclayton added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2017
@smarterclayton
Copy link
Contributor

Wanna bet?

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 41421, 41440, 36765, 41722)

@k8s-github-robot k8s-github-robot merged commit dfacc61 into kubernetes:master Feb 20, 2017
@liggitt liggitt deleted the watch-prefix branch February 20, 2017 19:08
@wojtek-t
Copy link
Member

Wanna bet?

:)

It seems that this PR broke our performance tests.
But looking into it, i think the issue is with the metrics, i.e. now we seem to be reporting "watch" requests as "LIST" in the prometheus.

I will see how easy is to fix this and if easy, i will fix it, otherwise I will revert this PR.

@wojtek-t
Copy link
Member

Hmm - I don't know how to easily fix it, so i'm going to revert it to unblock submit queue.

@wojtek-t wojtek-t mentioned this pull request Feb 21, 2017
Merged
@liggitt liggitt mentioned this pull request Feb 22, 2017
Merged
k8s-github-robot pushed a commit that referenced this pull request Feb 23, 2017
Merge pull request #41887 from liggitt/watch-verb
e5c2d71 
Automatic merge from submit-queue (batch tested with PRs 39855, 41433, 41567, 41887, 41652)

Use watch param instead of deprecated /watch/ prefix

Reopen of #41722 after reverted in #41774

Required #41797 to merge first

cc @deads2k @wojtek-t
@liggitt liggitt mentioned this pull request Mar 17, 2017
Closed
@liggitt liggitt mentioned this pull request Mar 25, 2017
Closed
perotinus pushed a commit to kubernetes-retired/cluster-registry that referenced this pull request Sep 2, 2017
Merge pull request #41887 from liggitt/watch-verb
bb1481b 
Automatic merge from submit-queue (batch tested with PRs 39855, 41433, 41567, 41887, 41652)

Use watch param instead of deprecated /watch/ prefix

Reopen of kubernetes/kubernetes#41722 after reverted in kubernetes/kubernetes#41774

Required kubernetes/kubernetes#41797 to merge first

cc @deads2k @wojtek-t
@liggitt liggitt mentioned this pull request Jun 15, 2018
Closed
@yue9944882 yue9944882 mentioned this pull request Feb 25, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mikedanese mikedanese

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.6
Development

Successfully merging this pull request may close these issues.
7 participants
@liggitt @k8s-reviewable @smarterclayton @k8s-github-robot @k8s-ci-robot @wojtek-t @mikedanese