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

Ensure not to use old concurrent-ruby #351

Merged

Conversation

y-yagi
Copy link
Contributor

@y-yagi y-yagi commented Aug 16, 2024

It seems that some users encounter with a negative values issue of Concurrent.available_processor_count.
#349 (comment)

To avoid the issue, this PR ensure not to use old concurrent-ruby. This PR also removes considering of NoMethodError because we can assume to use available_processor_count always.

It seems that some users encounter with a negative values issue of
`Concurrent.available_processor_count`.

grosser#349 (comment)

To avoid the issue, this PR ensure not to use old `concurrent-ruby`.
This PR also removes considering of `NoMethodError` because we can
assume to use `available_processor_count` always.
@@ -697,9 +697,10 @@ def instrument_start(item, index, options)
end

def available_processor_count
gem 'concurrent-ruby', '>= 1.3.4'
Copy link

Choose a reason for hiding this comment

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

What does this raise if ran under bundle exec with an older concurrent-ruby?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gem::LoadError will be raised. Gem::LoadError is a descendant of LoadError. So we can rescue it in

rescue LoadError, NoMethodError

@grosser grosser merged commit b24e422 into grosser:master Aug 16, 2024
6 checks passed
@grosser
Copy link
Owner

grosser commented Aug 16, 2024

thx! v1.26.3

@y-yagi y-yagi deleted the ensure_not_to_use_old_concurrent-ruby branch August 17, 2024 06:01
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