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.

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

Fix deprecated Faraday basic_auth #54

Merged
merged 2 commits into from
Mar 9, 2022

Conversation

estromlund
Copy link
Contributor

@estromlund estromlund commented Mar 2, 2022

Hi there 👋

We're using your library in our stack and have noticed that there are now warnings being thrown for usage of deprecated Faraday methods.

Specifically:

WARNING: `Faraday::Connection#basic_auth` is deprecated; it will be removed in version 2.0.
While initializing your connection, use `#request(:basic_auth, ...)` instead.
See https://lostisland.github.io/faraday/middleware/authentication for more usage info.

This PR updates the usage of basic_auth to be in line with the suggestion at that link.

I searched for other usages of basic_auth in here but this seems to be the only instance.

The specs ended up getting a little funky since this new approach adds the auth at the middleware level and not on the connection itself. This means that the Authorization headers don't exist on the connection, but are added once the request is in flight. I modified the specs to use an approach used by Faraday itself.

Please let me know if you have any questions and thanks for the consideration!

@ajrice6713
Copy link
Contributor

Hey @estromlund!

Thanks for taking the time to contribute to the project! We will take a look and run some tests to ensure everything looks good before merging in.

@ckoegel for visibility

@ckoegel
Copy link
Contributor

ckoegel commented Mar 9, 2022

Thanks for also including a test! @estromlund
This also revealed an issue with our activesupport dependency which will be fixed alongside this change. Everything looks great to me.

@ckoegel ckoegel merged commit 312e129 into Bandwidth:master Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants