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

Added QueryOptions for status api #7818

Merged
merged 2 commits into from
Jul 28, 2020
Merged

Added QueryOptions for status api #7818

merged 2 commits into from
Jul 28, 2020

Conversation

bogdanov1609
Copy link
Contributor

Added QueryOptions for status api, according to the documentation https://www.consul.io/api/status.html

hanshasselberg
hanshasselberg requested changes May 12, 2020
View reviewed changes
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

Nice catch! Could you add tests so that we don't run into this again?

@hanshasselberg hanshasselberg self-assigned this May 12, 2020
@hanshasselberg hanshasselberg added the waiting-reply Waiting on response from Original Poster or another individual in the thread label May 12, 2020
@bogdanov1609
Copy link
Contributor Author

Added the use of a variable Datacenter and two tests with wrong dc

@ghost ghost removed waiting-reply Waiting on response from Original Poster or another individual in the thread labels May 24, 2020
hanshasselberg
hanshasselberg approved these changes May 26, 2020
View reviewed changes
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your contribution!

@bogdanov1609
Copy link
Contributor Author

LGTM! Thank you for your contribution!

Let's merge then? :)

@bogdanov1609
Copy link
Contributor Author

@i0rek ping?

@Sinketsu
Copy link

Sinketsu commented Jul 2, 2020 •
edited
Loading

@i0rek
Hello!
May be merge it? This will be useful for my project!

@jsosulska jsosulska merged commit c34b088 into hashicorp:master Jul 28, 2020
@jsosulska
Copy link
Contributor

jsosulska commented Jul 28, 2020 •
edited
Loading

Hello All - Once an Approval is made, it is generally the original user who will merge in the change.
I misspoke above! I will merge this as it has been approved.

@mikemorris mikemorris mentioned this pull request Jul 28, 2020
Merged
@mikemorris
Copy link
Contributor

👋 Just wanted to update y'all that these functions have been renamed to LeaderWithQueryOptions(q *QueryOptions) and PeersWithQueryOptions(q *QueryOptions) in #8395 to avoid breaking the prior function signature.

Thanks for adding this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hanshasselberg hanshasselberg hanshasselberg approved these changes

Assignees

@hanshasselberg hanshasselberg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bogdanov1609 @Sinketsu @jsosulska @mikemorris @hanshasselberg