Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Fallback to sequentially fetching specs on 429s #6728

Merged
2 commits merged into from
Mar 8, 2019
Merged

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented Oct 8, 2018

What was the end-user problem that led to this PR?

The problem is that sometimes bundler is unable to resolve certain gemfiles. Specifically, sometimes it does not respect the required_ruby_version setting. This causes some people to assume that bundler will always try to install the latest version of any dependency, regardless of the ruby version being run, because as a matter of fact, this feature sometimes just doesn't work. See for example the discussion at https://github.com/rspec/rspec/issues/25.

The problem was consistently reproducible until a few minutes ago with the following Gemfile under ruby 2.3.7

source "https://rubygems.org"
ruby "2.3.7"
gem "berkshelf", "= 6.3.1"
$ docker run -it --rm --volume $(pwd):/app ruby:2.3.7 sh -c "cd /app && rm -f Gemfile.lock && bundle install"
Fetching gem metadata from https://rubygems.org/.............
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies....
Fetching public_suffix 3.0.3
Installing public_suffix 3.0.3
Fetching addressable 2.5.2
Installing addressable 2.5.2
Fetching buff-extensions 2.0.0
Installing buff-extensions 2.0.0
Fetching hashie 3.6.0
Installing hashie 3.6.0
Fetching varia_model 0.6.0
Installing varia_model 0.6.0
Fetching buff-config 2.0.0
Installing buff-config 2.0.0
Using bundler 1.16.5
Fetching fuzzyurl 0.9.0
Installing fuzzyurl 0.9.0
Fetching tomlrb 1.2.7
Installing tomlrb 1.2.7
Fetching mixlib-config 2.2.13
Installing mixlib-config 2.2.13
Fetching mixlib-shellout 2.4.0
Installing mixlib-shellout 2.4.0
Fetching chef-config 14.5.33
Installing chef-config 14.5.33
Fetching libyajl2 1.2.0
Installing libyajl2 1.2.0 with native extensions
Fetching ffi-yajl 2.3.1
Installing ffi-yajl 2.3.1 with native extensions
Fetching mixlib-log 2.0.4
Installing mixlib-log 2.0.4
Fetching rack 2.0.5
Installing rack 2.0.5
Fetching uuidtools 2.1.5
Installing uuidtools 2.1.5
Fetching chef-zero 14.0.6
Installing chef-zero 14.0.6
Gem::RuntimeRequirementNotMetError: chef-zero requires Ruby version >= 2.4.0. The current ruby version is 2.3.0.
An error occurred while installing chef-zero (14.0.6), and Bundler cannot continue.
Make sure that `gem install chef-zero -v '14.0.6' --source 'https://rubygems.org/'` succeeds before bundling.

In Gemfile:
  berkshelf was resolved to 6.3.1, which depends on
    chef was resolved to 14.5.33, which depends on
      chef-zero

Funny enough, I can no longer reproduce it at the moment, I guess it depends on the specific load conditions of the rubygems.org servers?

What was your diagnosis of the problem?

My diagnosis was that sometimes our resolution falls back to the old dependency API that didn't implement the required_ruby_version setting. In particular, this happens because the new API returns Net::HTTPTooManyRequests, so bundler gives up and defaults to the old API.

What is your fix for the problem, implemented in this PR?

My fix is to, instead of directly fall back to the old API when rate limiting happens, try first to fetch the dependencies sequentially instead of in parallel still from the new API, so that rate limit does not affect us.

Why did you choose this fix out of the possible options?

I chose this fix because it was the only idea that came up. As a matter of fact, #6471 and #6639 were closed because there was nothing we could do, so it seems like it's the only idea so far :)

@deivid-rodriguez
Copy link
Member Author

Failing on 1.8.7 as usual. Unsure whether to skip this for 1.8.7 or just let this PR hang in there until we drop support.

@indirect
Copy link
Member

indirect commented Oct 8, 2018

This seems like a great idea :)

@lamont-granquist
Copy link
Contributor

👍

@deivid-rodriguez
Copy link
Member Author

This should be good to go now that we've dropped 1.8 support! 😃

@deivid-rodriguez
Copy link
Member Author

My bad, I thought master had finally dropped support... I'll keep waiting :)

If the compact index returns TooManyRequests, take it easier by
requesting dependencies sequentially instead.
@deivid-rodriguez
Copy link
Member Author

I rebased this PR and it's now passing! 🎉

@indirect
Copy link
Member

indirect commented Mar 8, 2019

@bundlerbot r+

ghost pushed a commit that referenced this pull request Mar 8, 2019
6728: Fallback to sequentially fetching specs on 429s r=indirect a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem is that sometimes `bundler` is unable to resolve certain gemfiles. Specifically, sometimes it does not respect the `required_ruby_version` setting. This causes some people to assume that `bundler` will always try to install the latest version of any dependency, regardless of the ruby version being run, because as a matter of fact, this feature sometimes just doesn't work. See for example the discussion at https://github.com/rspec/rspec/issues/25.

The problem was consistently reproducible until a few minutes ago with the following `Gemfile` under `ruby 2.3.7`

```ruby
source "https://rubygems.org"
ruby "2.3.7"
gem "berkshelf", "= 6.3.1"
```

```
$ docker run -it --rm --volume $(pwd):/app ruby:2.3.7 sh -c "cd /app && rm -f Gemfile.lock && bundle install"
Fetching gem metadata from https://rubygems.org/.............
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies....
Fetching public_suffix 3.0.3
Installing public_suffix 3.0.3
Fetching addressable 2.5.2
Installing addressable 2.5.2
Fetching buff-extensions 2.0.0
Installing buff-extensions 2.0.0
Fetching hashie 3.6.0
Installing hashie 3.6.0
Fetching varia_model 0.6.0
Installing varia_model 0.6.0
Fetching buff-config 2.0.0
Installing buff-config 2.0.0
Using bundler 1.16.5
Fetching fuzzyurl 0.9.0
Installing fuzzyurl 0.9.0
Fetching tomlrb 1.2.7
Installing tomlrb 1.2.7
Fetching mixlib-config 2.2.13
Installing mixlib-config 2.2.13
Fetching mixlib-shellout 2.4.0
Installing mixlib-shellout 2.4.0
Fetching chef-config 14.5.33
Installing chef-config 14.5.33
Fetching libyajl2 1.2.0
Installing libyajl2 1.2.0 with native extensions
Fetching ffi-yajl 2.3.1
Installing ffi-yajl 2.3.1 with native extensions
Fetching mixlib-log 2.0.4
Installing mixlib-log 2.0.4
Fetching rack 2.0.5
Installing rack 2.0.5
Fetching uuidtools 2.1.5
Installing uuidtools 2.1.5
Fetching chef-zero 14.0.6
Installing chef-zero 14.0.6
Gem::RuntimeRequirementNotMetError: chef-zero requires Ruby version >= 2.4.0. The current ruby version is 2.3.0.
An error occurred while installing chef-zero (14.0.6), and Bundler cannot continue.
Make sure that `gem install chef-zero -v '14.0.6' --source 'https://rubygems.org/'` succeeds before bundling.

In Gemfile:
  berkshelf was resolved to 6.3.1, which depends on
    chef was resolved to 14.5.33, which depends on
      chef-zero
```

Funny enough, I can no longer reproduce it at the moment, I guess it depends on the specific load conditions of the rubygems.org servers?

### What was your diagnosis of the problem?

My diagnosis was that sometimes our resolution falls back to the old dependency API that didn't implement the `required_ruby_version` setting. In particular, this happens because the new API returns `Net::HTTPTooManyRequests`, so `bundler` gives up and defaults to the old API.

### What is your fix for the problem, implemented in this PR?

My fix is to, instead of directly fall back to the old API when rate limiting happens, try first to fetch the dependencies sequentially instead of in parallel still from the new API, so that rate limit does not affect us.

### Why did you choose this fix out of the possible options?

I chose this fix because it was the only idea that came up. As a matter of fact, #6471 and #6639 were closed because there was nothing we could do, so it seems like it's the only idea so far :)

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@ghost
Copy link

ghost commented Mar 8, 2019

Build succeeded

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants