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.

Sign up for GitHub

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

Jump to bottom

Koala breaks with faraday 2.0 release File does not exist: net/http/post/multipart #659

Open
1 task done
ericmustin opened this issue Jan 4, 2022 · 5 comments
Open
1 task done

Koala breaks with faraday 2.0 release File does not exist: net/http/post/multipart #659

ericmustin opened this issue Jan 4, 2022 · 5 comments

Comments

@ericmustin
Copy link

  • This is a Koala bug/issue/documentation problem.

Hello y'all. I help maintain Opentelemetry-ruby, which supports tracing instrumentation for the Koala gem. We noticed the Koala instrumentation test suite is broken in CI in opentelemetry-ruby, here's a test run thats failing

Relevant debug output

Installing faraday 2.0.0
Fetching koala 3.0.0
Installing koala 3.0.0
Bundle complete! 19 Gemfile dependencies, 43 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
opentelemetry-instrumentation-koala: appraisal
>> bundle check --gemfile='/home/runner/work/opentelemetry-ruby/opentelemetry-ruby/instrumentation/koala/gemfiles/koala_3.gemfile' || bundle install --gemfile='/home/runner/work/opentelemetry-ruby/opentelemetry-ruby/instrumentation/koala/gemfiles/koala_3.gemfile' --retry 1
Your Gemfile lists the gem koala (~> 3.0.0) more than once.
You should probably keep only one of them.
Remove any duplicate entries and specify the gem only once.
While it's not a problem now, it could cause errors if you change the version of one of them later.
Resolving dependencies...
The Gemfile's dependencies are satisfied
opentelemetry-instrumentation-koala: test
>> BUNDLE_GEMFILE=/home/runner/work/opentelemetry-ruby/opentelemetry-ruby/instrumentation/koala/gemfiles/koala_3.gemfile bundle exec rake test

File does not exist: net/http/post/multipart

So, net/http/post/multipart is a file from the gem https://github.com/socketry/multipart-post, which isn't required by koala, but instead it's required by faraday, a koala dependency.  However, the issue is that koala adds faraday as a dependency without any version constraint( see here). Faraday just cut a 2.0 release this morning(upgrade guide here), the 2.0 release is chok-ful-o breaking changes, including ripping out it's middleware into separate gems. One of those now "not included by default" middleware gems is faraday-multipart which is what brings in that socketry/multipart-post gem. (see here). Since that's not part of Faraday 2.0, koala raises an exception when it attempts to use that now missing nested dependency.

I am not sure the "right" way to fix this, but I imagine it's to either add a max version of <2.0 to this gemspec, or to add the relevant middleware from faraday that Koala is leveraging as a dependency as well.

Lmk if there's any more info needed on the above. Not super familiar with this library tbh, but happy to contribute a PR to fix this, just not sure what direction y'all want to take.
@ericmustin ericmustin mentioned this issue Jan 4, 2022
Merged
@ylecuyer
Copy link
Collaborator

ylecuyer commented Jan 4, 2022

Thanks for the really well detailed report, I'll have a look at what can be done here, meanwhile your workarround setting a max version of < 2 for faraday will allow you to continue using the gem

@ylecuyer
Copy link
Collaborator

ylecuyer commented Jan 4, 2022 •
edited
Loading

I'd suggest the following:

  1. add faraday < 2 to the runtime dependency and release a 3.1.0
    2.1 Remove the multipart hack introduced in OAuthException: (#100) properties must be a dictionary #125 I tried and couldn't reproduce. I'll do more testing tomorrow just to be sure
    2.2 Upgrade the gem to be compatible with faraday 2 following their upgrading guide https://github.com/lostisland/faraday/blob/main/UPGRADING.md
    2.3 add faraday > 1, < 3 to the runtime dependency and release a 4.0.0

I do not think it is worth having koala supporting both farday 1 and farday 2 at the same time. The gem is well done and does not need updates when facebook releases new API versions. If a security fix is needed or a feature really needs to be added, backporting to the 3.x should be easier.

@arsduo WDYT? Would you be able to help with the releasing process on rubygems?

ylecuyer added a commit to ylecuyer/koala that referenced this issue Jan 16, 2022
@ylecuyer
Compatibility: Lock Faraday to < 2. arsduo#659
8c2521a
@ylecuyer ylecuyer mentioned this issue Jan 16, 2022
Merged
ylecuyer added a commit that referenced this issue Jan 17, 2022
@ylecuyer
Compatibility: Lock Faraday to < 2. (#659)
021a6aa
@ylecuyer ylecuyer mentioned this issue Jan 17, 2022
Merged
@ylecuyer
Copy link
Collaborator

Part 1 of the plan is ready and release 3.1.0 should be out soon (#663)

@ericmustin I checked with the opentelemetry projects and everything is OK https://github.com/ylecuyer/opentelemetry-ruby/runs/4845955846?check_suite_focus=true and https://github.com/ylecuyer/opentelemetry-ruby/pull/1/files

regarding the plan, I also added 3.x support so that we don't have to do another release when ruby 3 will be mainstream

Later this week I'll work on the part 2 of the plan adding faraday 2 support

@ylecuyer ylecuyer mentioned this issue Jan 18, 2022
Closed
@ericmustin
Copy link
Author

ok, sounds good, thanks for the quick help here!

@jimtng
Copy link

jimtng commented Jul 5, 2022

I came across this issue just now. Another gem needs Faraday 2, so I had to downgrade koala to 3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ylecuyer @jimtng @ericmustin