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

fix: update http gem to 4.4.0 #110

Conversation

MosesMendoza
Copy link

This PR is related to IBM/ruby-sdk-core#33

I was integrating with the Ruby SDK in a rails app, which runs ruby 2.7.2. I wasn't able to initialize an IamAuthenticator object in that lib and traced it down to the http gem. Relevant stack trace portion:

[5] pry(#<Api::V1::AnalysesController>)> exception.backtrace
=> ["/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/http-4.1.1/lib/http/response/body.rb:52:in `force_encoding'",
 "/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/http-4.1.1/lib/http/response/body.rb:52:in `to_s'",
 "/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/ibm_cloud_sdk_core-1.1.2/lib/ibm_cloud_sdk_core/token_managers/jwt_token_manager.rb:85:in `request'",
 "/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/ibm_cloud_sdk_core-1.1.2/lib/ibm_cloud_sdk_core/token_managers/iam_token_manager.rb:54:in `request_token'",
 "/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/ibm_cloud_sdk_core-1.1.2/lib/ibm_cloud_sdk_core/token_managers/jwt_token_manager.rb:30:in `token'",
 "/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/ibm_cloud_sdk_core-1.1.2/lib/ibm_cloud_sdk_core/token_managers/iam_token_manager.rb:33:in `initialize'",
 "/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/ibm_cloud_sdk_core-1.1.2/lib/ibm_cloud_sdk_core/authenticators/iam_authenticator.rb:28:in `new'",
 "/Users/moses/development/foo/vendor/bundle/ruby/2.7.0/gems/ibm_cloud_sdk_core-1.1.2/lib/ibm_cloud_sdk_core/authenticators/iam_authenticator.rb:28:in `initialize'",

I came across this fix in the http gem repo that references the frozen string error. It was backported to http gem version 4.3.0. This PR updates the gem dependency, but it conflicted with the dependency in the ruby-sdk-core repo so I updated that one as well in this PR. After this fix (and the one in that repo) the code ran fine in rails on ruby 2.7.2 I'm running the project from gem sources in github at the moment, but here's a PR in case you want to update officially. This PR changes the dep to 4.4.x, but 4.1 -> 4.4 should be backwards-compatible. I'm not an expert at minitest but it seems all the tests pass with these changes (at least the ones triggered by bundle exec rake.

Cheers

Signed-off-by: Moses Mendoza <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Feb 24, 2021

CLA assistant check
All committers have signed the CLA.

@jeff-arn
Copy link

@MosesMendoza thanks for opening this contribution! It looks like we would also need to bump the http dep in the ruby core, so we will look into that and see if we can update the core to resolve the build issues here.

@@ -35,7 +35,7 @@ Gem::Specification.new do |spec|
spec.add_runtime_dependency "concurrent-ruby", "~> 1.0"
spec.add_runtime_dependency "eventmachine", "~> 1.2"
spec.add_runtime_dependency "faye-websocket", "~> 0.11"
spec.add_runtime_dependency "http", "~> 4.1.0"
spec.add_runtime_dependency "http", "~> 4.4.0"
spec.add_runtime_dependency "ibm_cloud_sdk_core", "~> 1.1.1"
Copy link

Choose a reason for hiding this comment

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

This would need to specify the latest core version if we merged it into the core.

@Mikemosca Mikemosca closed this Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants