Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: #186 token as request param is deprecated #245

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mehiel
Copy link

@mehiel mehiel commented May 5, 2023

Consul now warns aggressively when token is provided as a request param and the X-Consul-Token is required. Also as already mentioned in #186 token request param will be removed in Consul v1.17.

By reading the code and related PRs I've seen that in the so called "new http architecture" this is addressed although not all clients fully utilize the new Request class.

This PR uses the Request class for both Catalog and KV clients.

Should address both #186 and #237.

Consul now warns aggressively when token is provided as a request param
and the X-Consul-Token is required. Also as already mentioned in Ecwid#186
token request param will be removed in Consul v1.17.

By reading the code and related PRs I've seen that in the so called
"new http architecture" this is addressed although not all clients fully
utilize the new Request class.

This PR uses the Request class for both Catalog and KV clients.

Should address both Ecwid#186 and Ecwid#237.
@mehiel
Copy link
Author

mehiel commented May 5, 2023

There is also #132 and other token related issues which may need some more effort but it's feasible to get them fixed.

If the maintainers (@vgv ?) are interested I could make another PR and try to address as much clients and API operations as possible.

Copy link

@linghengqian linghengqian left a comment

Choose a reason for hiding this comment

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

  • Call @vgv to review.

@mehiel
Copy link
Author

mehiel commented Jan 31, 2024

Should we declare this project abandoned?

@linghengqian
Copy link

Should we declare this project abandoned?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants