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

Address Faraday warnings on v0.x #971

Merged
merged 1 commit into from
Mar 10, 2020
Merged

Address Faraday warnings on v0.x #971

merged 1 commit into from
Mar 10, 2020

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Mar 9, 2020

Fixes #965

When we added support for Faraday v1 in #906, a warning started to be emitted by Faraday versions < 1.0: WARNING: Unexpected middleware set after the adapter. This won't be supported from Faraday 1.0.
This did not affect the correct behaviour of instrumentation on Faraday < 1.0.

This PR addresses that warning by patching Faraday < 1.0 using our old approach, the one removed in #906, instead of the new patching used by v1.0.

The removal of the old approach was originally intended, as the new patching used for 1.0 also works for < 1.0, as stated, but this warning we are seeing is not desirable either.

I've added a new Appraisal category for contrib to allow us to test older version: contrib-old. I'm using it to test Faraday 0.17, while keeping our regular tests of the latest version of Faraday under contrib.

@marcotc marcotc added bug Involves a bug integrations Involves tracing integrations labels Mar 9, 2020
@marcotc marcotc requested a review from a team March 9, 2020 22:17
@marcotc marcotc self-assigned this Mar 9, 2020
@delner
Copy link
Contributor

delner commented Mar 10, 2020

The removal of the old approach was originally intended, as the new patching used for 1.0 also works for < 1.0, as stated, but this warning we are seeing is not desirable either.

I'm not so clear on this... changing the patching strategy in #906 to support 1.0 works for 1.0, but when present in versions < 1.0 causes that older version to raise a warning that it won't work in 1.0? Despite it actually working in 1.0?

@marcotc
Copy link
Member Author

marcotc commented Mar 10, 2020

I'm not so clear on this... changing the patching strategy in #906 to support 1.0 works for 1.0, but when present in versions < 1.0 causes that older version to raise a warning that it won't work in 1.0? Despite it actually working in 1.0?

@delner this is accurate. I'm guessing they didn't predict exactly how 1.0 would be implemented, so the warning gets tripped even though it works.

Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

👍 Thanks for the fix, @marcotc !

@marcotc marcotc merged commit 1d31eaa into master Mar 10, 2020
@marcotc marcotc added this to Merged & awaiting release in Active work via automation Mar 10, 2020
@marcotc marcotc deleted the fix/faraday-v0-warning branch March 10, 2020 20:09
@delner delner added this to the 0.34.0 milestone Mar 30, 2020
@delner delner moved this from Merged & awaiting release to Released in Active work Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug integrations Involves tracing integrations
Projects
Active work
  
Released
Development

Successfully merging this pull request may close these issues.

Warnings when using Faraday adapter with Farday 0.17.3
2 participants