lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.75k stars 981 forks source link

Use external multipart and retry middleware #1367

Closed iMacTia closed 2 years ago

iMacTia commented 2 years ago

Description

Use external multipart and retry middleware. It should allow external gems to support both Faraday v1 and v2

Additional notes

I had to bump Ruby min version to 2.6, but considering 2.6 is going EOL at the end of March, I guess that should be fine?

javierjulio commented 2 years ago

I upgraded to Faraday v1.8 and implemented something new to use Faraday::ParamPart and Faraday::FilePart which worked (I'm new to using Faraday) but with this change our builds failed on a Dependabot update for Faraday v1.9.x which was unexpected.

Reviewing this, I remember now about the work around moving parts into external gems. If I understand right, would it be expected that I just have to now use Faraday::Multipart::ParamPart and Faraday::Multipart::FilePart (source) going forward? I'm already setting f.request :multipart (source) equivalent as expected.

iMacTia commented 2 years ago

@javierjulio those are the correct classes to use from now on, but the old one should have worked just the same! That was a regression in the first "standalone" version of faraday-multipart, which have now been fixed in the last release (1.0.2).

Could you try to check which version of the middleware you're using and if upgrading to the latest fixes the issue?

javierjulio commented 2 years ago

Thank you! In the meantime I'll make that update to use the Faraday::Multipart namespace versions instead.

Yes I can. I've confirmed the faraday-multipart gem was updated to 1.0.2. Below is the diff from the Dependabot PR we received to bump faraday from 1.8.0 to 1.9.3 and where our tests failed with the uninitialized constant Faraday::ParamPart error. That precedes the line using the Faraday::FilePart by the way, so I would assume the same error would occur there.

-    faraday (1.8.0)
+    faraday (1.9.3)
       faraday-em_http (~> 1.0)
       faraday-em_synchrony (~> 1.0)
       faraday-excon (~> 1.1)
-      faraday-httpclient (~> 1.0.1)
+      faraday-httpclient (~> 1.0)
+      faraday-multipart (~> 1.0)
       faraday-net_http (~> 1.0)
-      faraday-net_http_persistent (~> 1.1)
+      faraday-net_http_persistent (~> 1.0)
       faraday-patron (~> 1.0)
       faraday-rack (~> 1.0)
-      multipart-post (>= 1.2, < 3)
+      faraday-retry (~> 1.0)
       ruby2_keywords (>= 0.0.4)
     faraday-em_http (1.0.0)
     faraday-em_synchrony (1.0.0)
     faraday-excon (1.1.0)
     faraday-httpclient (1.0.1)
+    faraday-multipart (1.0.2)
+      multipart-post (>= 1.2, < 3)
     faraday-net_http (1.0.1)
     faraday-net_http_persistent (1.2.0)
     faraday-patron (1.0.0)
     faraday-rack (1.0.0)
+    faraday-retry (1.0.3)
     faraday_middleware (1.2.0)
       faraday (~> 1.0)
iMacTia commented 2 years ago

I just reviewed the code and it seems I missed ParamPart in the aliases. I'll add it back as well and will ship a release 1.0.3 with it 👍 Thanks for raising this!

iMacTia commented 2 years ago

@javierjulio That's done 🙌. Once again feedback welcome if the fix works 🙏

javierjulio commented 2 years ago

@iMacTia you're welcome! I just ran our test against the 1.0.3 release, without any of the changes I mentioned before, and it passed so its fixed. Thank you!

To confirm, since we'd like to be ready for Faraday v2, we are now adding gem 'faraday-multipart' to our Gemfile and then using Faraday::Multipart namespace to access the ParamPart and FilePart classes instead.

iMacTia commented 2 years ago

@javierjulio wonderful!

To confirm, since we'd like to be ready for Faraday v2, we are now adding gem 'faraday-multipart' to our Gemfile and then using Faraday::Multipart namespace to access the ParamPart and FilePart classes instead.

That's great 🙌! Just bear in mind that if you want to support both Faraday v1 and v2 you'll need a few conditionals in your code. I'm still trying to find the time to put a guide together, but in short that would be: