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

Add API support for cancelation contexts passed via QueryOptions and WriteOptions #8836

Merged
merged 3 commits into from
Sep 9, 2020
Merged

Add API support for cancelation contexts passed via QueryOptions and WriteOptions #8836

merged 3 commits into from
Sep 9, 2020

Conversation

benbuzbee
Copy link
Contributor

@benbuzbee benbuzbee commented Sep 7, 2020 •
edited
Loading

Fixes #8009
Copy Consul API's format: QueryOptions.WithContext(context) will now return a new QueryOption whose HTTP requests will be canceled with the context provided, and the same for WriteOptions

Add API support for cancelation context's passed via QueryOptions
4d8c6a2 
Copy Consul API's format: QueryOptions.WithContext(context) will now return a new QueryOption whose HTTP requests wiill be canceled with the context provided.
tgross
tgross requested changes Sep 8, 2020
View reviewed changes
api/api.go Show resolved Hide resolved
api/api.go Show resolved Hide resolved
api/api.go Show resolved Hide resolved
api/api.go Show resolved Hide resolved
@benbuzbee
Copy link
Contributor Author

@tgross I've replied with rationale for all comments, with that info I will make whatever changes you would like to see just let me know here, thanks!

@benbuzbee benbuzbee changed the title Add API support for cancelation contexts passed via QueryOptions Add API support for cancelation contexts passed via QueryOptions and WriteOptions Sep 9, 2020
@benbuzbee
Copy link
Contributor Author

Could use some advice on the tests. Before i made the latest updated they apssed, now apparently 445 fail - doesnt seem related but...

@tgross
Copy link
Member

tgross commented Sep 9, 2020

Could use some advice on the tests. Before i made the latest updated they apssed, now apparently 445 fail - doesnt seem related but...

It looks like this is a not-as-rare-as-we'd-like issue we've seen in CI where the test runner gets locked up and times out. I kicked-off the CI job again and the tests pass now.

tgross
tgross approved these changes Sep 9, 2020
View reviewed changes
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@tgross tgross merged commit 648140a into hashicorp:master Sep 9, 2020
@tgross tgross modified the milestones: unscheduled , 0.12.4, 0.13 Sep 9, 2020
@tgross tgross mentioned this pull request Sep 9, 2020
Closed
tgross added a commit that referenced this pull request Sep 18, 2020
@tgross
changelog items for community PRs
6cdf23b 
[ci skip]

* #8836
* #8921
@tgross tgross mentioned this pull request Sep 18, 2020
Merged
tgross added a commit that referenced this pull request Sep 18, 2020
@tgross
changelog items for community PRs (#8924)
3a150d3 
[ci skip]

* #8836
* #8921
@notnoop
Copy link
Contributor

notnoop commented Sep 18, 2020

Thanks @benbuzbee for this PR! This is awesome and would have been handy for me in few occasions. Now, I wish we could pass Context for server-to-server RPC communications.

roaks3 pushed a commit that referenced this pull request Oct 7, 2020
@tgross @roaks3
changelog items for community PRs (#8924)
d2c95eb 
[ci skip]

* #8836
* #8921
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cgbaker cgbaker cgbaker left review comments

@tgross tgross tgross approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.

Ability to cancel blocking queries from the API client
4 participants
@benbuzbee @tgross @notnoop @cgbaker