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

CI: Use same Ruby version matrix as Ruby, repair for Faraday 1.0 #228

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Apr 24, 2020

This PR wants to use latest released Faraday.
⚠️ This is a work in progress. Some bits remain. Will update this Description when finalized.

Background

CI changes

This PR adds new Ruby versions and removes old ones.

The Ruby support schedule currently contains 2.5-2.7. https://www.ruby-lang.org/en/downloads/branches/ - and, people who are using this gem to communicate with Parse probably run those supported Ruby versions.

This PR also removes the no-longer-used Travis setting sudo: false. See more at the Travis blog.

Faraday middleware change

The Retry middleware changed enough upstream in 1.0 that the local overrides here make it troublesome.

Details

  • Faraday 1.0 changed namespace on some of the classes
  • The retry middleware changed, a little, too

@coveralls
Copy link

Coverage Status

Coverage decreased (-97.2%) to 0.0% when pulling 92a081d on olleolleolle:patch-1 into 323e931 on adelevie:master.

  - BetterRetry has a frozen array, workaround for that
@olleolleolle olleolleolle changed the title CI: Use same Ruby version matrix as Ruby CI: Use same Ruby version matrix as Ruby (WIP) Apr 24, 2020
@olleolleolle olleolleolle changed the title CI: Use same Ruby version matrix as Ruby (WIP) CI: Use same Ruby version matrix as Ruby, repair for Faraday 1.0 Apr 27, 2020
Copy link
Contributor

@henrik henrik left a comment

Choose a reason for hiding this comment

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

I work on the same team as @olleolleolle and he asked me to have a look :)

@@ -12,11 +12,11 @@ def initialize(app, options = nil)
super(app, options)

# NOTE: Faraday 0.9.1 by default does not retry on POST requests
@options.methods << :post
@options.methods = @options.methods + [:post]
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: Could use += but it's not objectively better or anything :)

@@ -33,8 +33,10 @@ def call(env)
log(env, exception)
retries -= 1
retries_header(env, retries)
sleep sleep_amount(retries + 1)
retry
if (sleep_amount = calculate_sleep_amount(retries + 1, env))
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is calculate_sleep_amount defined? Don't see 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.

It's in the Faraday codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps it's better to do a full replace of the retry middleware - than to patch this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, of course.

@olleolleolle olleolleolle marked this pull request as draft May 5, 2020 04:40
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.

3 participants