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

Bump faraday version to 0.12.2 #797

Merged
merged 2 commits into from May 13, 2019
Merged

Bump faraday version to 0.12.2 #797

merged 2 commits into from May 13, 2019

Conversation

guicassolato
Copy link
Contributor

@guicassolato guicassolato commented May 10, 2019

What this PR does / why we need it
It updates the version of the faraday gem in use to v0.12.2.

Which issue(s) this PR fixes
This is expected to include support to no_proxy env var in the HTTP requests performed using the gem, such as connections to backend, segment and some requests performed by Sidekiq.

Closes THREESCALE-1739

@codecov
Copy link

codecov bot commented May 10, 2019

Codecov Report

Merging #797 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #797      +/-   ##
==========================================
- Coverage    92.9%   92.89%   -0.01%     
==========================================
  Files        2414     2414              
  Lines       78251    78251              
==========================================
- Hits        72696    72693       -3     
- Misses       5555     5558       +3
Impacted Files Coverage Δ
test/test_helpers/backend.rb 71.83% <0%> (ø) ⬆️
lib/tasks/multitenant/tenants.rake 53.84% <0%> (-23.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05adee1...c11b0f9. Read the comment docs.

@guicassolato guicassolato changed the title Bump faraday version to 0.12.0 Bump faraday version to 0.12.2 May 10, 2019
josemigallas
josemigallas previously approved these changes May 13, 2019
@@ -85,7 +85,7 @@ def raise_on_method(klass, *methods)
MockCore.mock_core!

def stub_core_integration_errors(service_id: )
MockCore.stubs.get("/internal/services/#{service_id}/errors/", {'User-Agent' => 'Faraday v0.9.2'}) do
MockCore.stubs.get("/internal/services/#{service_id}/errors/", {'User-Agent' => 'Faraday v0.12.2'}) do
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this header?
I think it does not make any harm to remove it. @3scale/system

Copy link
Contributor

Choose a reason for hiding this comment

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

This is for tests 🤔 We can try to remove it and if all tests pass, all good 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think Faraday sets it accordingly when omitted: https://github.com/lostisland/faraday/blob/v0.12.2/lib/faraday/connection.rb#L102

Now we could also use Faraday.default_connection_options if we're not really aiming to assert anything that could differ based on the User-Agent: https://stackoverflow.com/questions/45622842/globally-configure-faraday-user-agent

Copy link
Contributor

Choose a reason for hiding this comment

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

Well I do not see the point of testing this header.
If it was on purpose then I do not know the purpose of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests fail if a fixed User-Agent is provided and it mismatches the actual Faraday version.

Martouta
Martouta previously approved these changes May 13, 2019
Faraday sets the User-Agent header according to the version of the gem
if not specified: https://github.com/lostisland/faraday/blob/v0.12.2/lib/faraday/connection.rb#L102
@guicassolato guicassolato dismissed stale reviews from Martouta and josemigallas via c11b0f9 May 13, 2019 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants