Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Faraday Wishlist #953

Closed
12 of 32 tasks
technoweenie opened this issue Mar 20, 2019 · 18 comments
Closed
12 of 32 tasks

Faraday Wishlist #953

technoweenie opened this issue Mar 20, 2019 · 18 comments

Comments

@technoweenie
Copy link
Member

technoweenie commented Mar 20, 2019

I want to start collecting a wishlist for what we want to change in Faraday. Nothing here is concrete yet.

Faraday v2.0

  • Reorganize param encoders: lib/faraday/params_encoders/[nested, flat] ?
  • Extract adapters as gems
    • em-http
    • em-http-ssl-patch
    • em-synchrony
    • excon
    • httpclient
    • net-http
    • net-http-persistent
    • patron
    • rack
    • typhoeus

Faraday v3.0

  • Use SNAKE_CASE constants consistently
  • Kill Faraday::Utils
  • Faraday::Connection => Faraday::Client
  • Remove Options structs in favor of properties on Client/Request/Response
  • Remove env in favor of request/response object properties
  • Consistent #response property on HTTP-related error classes (RaiseError, RetriableRequest, etc) [Can not view details of failed requests  #1284]
  • Kill adapter/middleware autoloading in favor of good ole ruby
  • Revisit adapter/middleware internal API (drop Rack adapter semantics)
  • Redesign Faraday::Connection and Faraday::RackBuilder relationship
    • Remove use/adapter/etc delegation
    • Faraday::RackBuilder#handlers => Faraday::Connection#handlers
  • Reorganize middleware lib/faraday/middleware/*
  • Retry mw: extract exponential backoff timing stuff
  • Logging/Instrumentalization: built into Faraday, usable by any middleware
  • Combine net/http and net/http persistent adapters
  • Streaming by default, while providing easy access to cached string response bodies
  • HTTP/Socks proxy support (needs to be implemented in the http libs themselves)
  • Built-in multipart support with better api.
  • Revisit pipelining or parallel requests (net-http-pipeline, typhoeus)
  • Consistent response error raising (/cc #1042)
@grosser
Copy link
Contributor

grosser commented May 25, 2019

  • make net-http-persistent less hacky by keeping a connection object around so we don't need a global cache
  • make net-http-persistent not use the gem ... 90% of it is managing net-http which we already do, so the only thing we need is the rescue + reopen logic which is a few lines of code and would remove coupling/translation logic for the gem see remove retry logic drbrain/net-http-persistent#100
  • allow using net-http-pipeline

@technoweenie
Copy link
Member Author

Thanks, those are great suggestions!

make net-http-persistent less hacky by keeping a connection object around so we don't need a global cache

Ah yes, another reason why Faraday's implementation of Rack semantics for the adapter and middleware classes needs to go. If the current adapter was a long lived Faraday::Connection#adapter property, the net-http adapter could hold on to the connection object. I just added "Revisit adapter/middleware internal API (drop Rack adapter semantics)" to the wishlist to support this.

make net-http-persistent not use the gem

I'm on board. Thanks for the pointer to the PR.

allow using net-http-pipeline

Faraday does support parallel requests, but I'm not sure offhand if we could use net-http-pipeline to implement them for net-http. I've added "Revisit pipelining or parallel requests (net-http-pipeline, typhoeus)" to the wishlist.

@grosser
Copy link
Contributor

grosser commented May 31, 2019 via email

@iMacTia
Copy link
Member

iMacTia commented Dec 31, 2020

@grosser you might have heard we're now in the process of pushing adapters and middleware out of Faraday.
We have already done a lot of work to make this as easy as possible, including exporting tests and providing a few examples (faraday-net_http faraday-http)

I was wondering, given your past contributions, if you'd like to take ownership of net_http_persistent?
All you'd need to do is to isolate it into a separate repo (this can be under your user!) as we did for the examples above, and release a v1.0 which is exactly the same as the current one. We'll then add it to the Faraday gem spec for backwards-compatibility. The plan is then to drop these dependencies for Faraday v2.0

From there, you're free to change/refactor it to you heart's content and making all the breaking changes you want 😄
Please let me know if you're interested or not 🙌! I'm also happy to help with the first migration as I'm planning to do that work anyway

@grosser
Copy link
Contributor

grosser commented Dec 31, 2020 via email

@iMacTia
Copy link
Member

iMacTia commented Jan 4, 2021

@grosser Awesome! Please shout out if you need any help!
@julik please see my comment above for @grosser, would you like to do something similar for the Patron adapter 😄?

@julik
Copy link
Contributor

julik commented Jan 5, 2021

Hey folks, happy 2021! I'm fine taking ownership of the Patron adapter, the problem I have encountered when setting it up though was that I found the use of mocks in the shared tests very difficult to manage (also given that webmock patches Patron in a few places, without that really being called for). When doing the extraction I stumbled into the fact that I would then also have to become a co-maintainer of the Patron webmock overrides - that's one thing, and that instead of testing whether the thing that needs to work actually works I would be testing whether it works with Webmock. This is a bit personal, but a relatively difficult subject for 2020 for me has been having to convince people of things, and I depleted my "convincing" budget for that year. And heavily went into overdraft, and it actually became a bit of a risk for my well-being 😄 I can get from A to B in a very safe and effective manner, but the amount of back-and-forth I can handle along that path is way lower than it used to be. This also has to do with the fact that I am part of an org experiencing fast growth. I am in no position to demand that concessions be made for this – it is my personal issues after all. But I have to budget my involvement in things.

So: If we can revisit that decision (having to use webmock instead of actual requests, which is what the adapters are about) I'm fine taking ownership of the patron integration wholesale.

@iMacTia
Copy link
Member

iMacTia commented Jan 5, 2021

@julik totally get your point! I remember we discussed already in the past the main reason for introducing Webmock, and having to deal with 8 different adapters was definitely one of those!

We've made tests available externally so that the migration from bundled to external adapters would be as smooth and easy as possible, and since Faraday v1.0 will still bundle adapters (but included as gems, rather than files in the lib folder), then it makes sense to use them as they are when extracting the repo into a new gem.

BUT THAT'S IT! Once the v1.0 of the adapter gem has been created and added to Faraday for backwards-compatibility, like we did for the Net::HTTP adapter, that's where you can start a v2.0 path for the adapter gem and decide what to do with it.
That, of course, includes rewriting tests with whatever framework you like and using real calls, if you fancy.
Once the gem is in the so-called "user land" we have absolutely no right to call the shots anymore and all decisions should be up to the community and the gem owners.

And I'll tell you more, we're already discussing internally about creating some sort of "integration tests" suite with actual requests being made. The main features of this suite would be (all still up for discussion):

  • Gemified/Packaged so that it can be used "plug-and-play" with any adapter
  • Makes real requests
  • Support for docker containers (to help with things like a mock API and proxy servers)
  • Performance testing
  • Fibers/Concurrency support
  • Features check report (tests against "Faraday" base features and produces a report for the adapter to show which ones are supported by the adapter, useful for users who are looking for a new one)
  • Open for more... !

@technoweenie has even started working on that: https://github.com/technoweenie/faraday-live

So, if you'd fancy helping out with that as well, as I remember you had some cool ideas on how that might work, we'd also welcome inputs and help on that front 😃

@MikeRogers0
Copy link
Contributor

I'm also happy to help where I can (Thank you @olleolleolle for showing me this) :)

I saw https://github.com/lostisland/faraday/projects/3 - In terms of splitting out these into gems, are there plans to create a faraday organisation on GitHub to store all the individual gems?

@olleolleolle
Copy link
Member

@iMacTia, a cc for you

@MikeRogers0 lostisland, this org, is home to faraday-http, a gemified adapter. Perhaps more of the adapters just live in this org?

@MikeRogers0
Copy link
Contributor

Awesome! I totally blanked last night that @lostisland was an organisation account :) I think putting them in the organisation is my preference.

Next week I'll look through what was changed to make move the Net::HTTP adapter into a gem, then duplicate for Net::HTTP::Persistent :) To help expedite things I'll start in my personal account, once it looks like it has taken shape I'll put a link up & we can transfer it over.

@iMacTia
Copy link
Member

iMacTia commented Mar 6, 2021

Thanks @MikeRogers0, that is wonderful to hear, any extra help is much appreciated.
@grosser is also quite knowledgeable on the Net::HTTP::Persistent adapter having contributed to it in the past so please feel free to keep him in the loop.

As for the location of the adapters, I personally don't feel strongly about where they should live.
It makes perfect sense to me for them to live under a person personal account, if that person is the main maintainer as well.
So if you're happy with that, I have nothing against leaving the adapter under your account 😄

@iMacTia
Copy link
Member

iMacTia commented Mar 6, 2021

@MikeRogers0 @julik @grosser I've created a new template repo to make creating a new adapter easier: https://github.com/lostisland/faraday-adapter-template

Please consider this very much like a WIP and feel free to provide any feedback to improve it!

@MikeRogers0
Copy link
Contributor

@iMacTia That template repo was very handy!

I set up https://github.com/MikeRogers0/faraday-net_http_persistent based on it - I did set it up to be transferred to @lostisland - Do you want to eyeball it & make sure I've not missed anything obvious? :)

@iMacTia iMacTia pinned this issue Mar 12, 2021
@iMacTia
Copy link
Member

iMacTia commented Mar 13, 2021

Fantastic work @MikeRogers0 🎉, and really quick as well!
I was checking how repository transfer works and it's a bit more complicated when going from user to organisation, so if you're still willing to transfer that into `lostisland, could you please transfer it over to me first so then I'll transfer it in myself? I'll then add you and @grosser as maintainers to that repo 👍

Also, glad to hear the template repo was helpful! If you have any feedback (anything unclear, anything you wished was in there, typos, etc...) please let me know or feel free to open a PR against it 😄

Next steps would be to release a first version of the gem into Rubygems, remove the Net::HTTP::Persistent adapter from Faraday and plug your new gem in there.
I can take care of the releasing, up to you if you want to do the swapping PR against Faraday or not (here the PR on how we did it for Net::HTTP)

@MikeRogers0
Copy link
Contributor

MikeRogers0 commented Mar 13, 2021

image

@iMacTia - Awesome, transfer started :)

Also, glad to hear the template repo was helpful! If you have any feedback

I did add a GitHub Action in & rewrote the Readme. I PR'ed the main changes I made which I think could be useful :) I did also swap out Rubocop for StandardRB, which I'd like to encourage.

Next steps would be to release a first version of the gem into Rubygems, remove the Net::HTTP::Persistent adapter from Faraday and plug your new gem in there.

Awesome! I'll get working on a PR 🎉

@iMacTia
Copy link
Member

iMacTia commented Mar 14, 2021

@MikeRogers0 @grosser repo transferred and you should both have received an invitation 👍
https://github.com/lostisland/faraday-net_http_persistent

@MikeRogers0 thanks for the feedback 🙏!

@iMacTia
Copy link
Member

iMacTia commented Mar 15, 2021

@MikeRogers0 faraday-net_http_persistent is now available on Rubygems 🎉
https://rubygems.org/gems/faraday-net_http_persistent

@iMacTia iMacTia changed the title v2 Wishlist Faraday Wishlist Jul 31, 2021
@iMacTia iMacTia removed this from the v2.0 milestone Jul 31, 2021
@lostisland lostisland locked and limited conversation to collaborators Oct 9, 2021
@iMacTia iMacTia closed this as completed Oct 9, 2021
@iMacTia iMacTia unpinned this issue Oct 14, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants