-
Notifications
You must be signed in to change notification settings - Fork 981
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
Comments
|
Thanks, those are great suggestions!
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
I'm on board. Thanks for the pointer to the PR.
Faraday does support parallel requests, but I'm not sure offhand if we could use |
For pipeline: I decided to not use that in my projects because it means
rewriting most of the handler logic, so low prio for me
…On Fri, May 31, 2019 at 10:31 AM risk danger olson ***@***.***> wrote:
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.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#953?email_source=notifications&email_token=AAACYZ5IS7IRWR45K7IFKL3PYFOILA5CNFSM4HAAQSK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWV4EOI#issuecomment-497795641>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAACYZ6EKR7M4ARR47IAI23PYFOILANCNFSM4HAAQSKQ>
.
|
@grosser you might have heard we're now in the process of pushing adapters and middleware out of Faraday. I was wondering, given your past contributions, if you'd like to take ownership of net_http_persistent? From there, you're free to change/refactor it to you heart's content and making all the breaking changes you want 😄 |
sounds easy, I'll try it
…On Thu, Dec 31, 2020 at 4:03 AM Matt ***@***.***> wrote:
@grosser <https://github.com/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.
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
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#953 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAACYZYOKFNQGJM5GXZQJMLSXRR73ANCNFSM4HAAQSKQ>
.
|
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. |
@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 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):
@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 😃 |
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? |
@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? |
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 |
Thanks @MikeRogers0, that is wonderful to hear, any extra help is much appreciated. As for the location of the adapters, I personally don't feel strongly about where they should live. |
@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! |
@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? :) |
Fantastic work @MikeRogers0 🎉, and really quick as well! 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 |
@iMacTia - Awesome, transfer started :)
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.
Awesome! I'll get working on a PR 🎉 |
@MikeRogers0 @grosser repo transferred and you should both have received an invitation 👍 @MikeRogers0 thanks for the feedback 🙏! |
@MikeRogers0 |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
I want to start collecting a wishlist for what we want to change in Faraday. Nothing here is concrete yet.
Faraday v2.0
lib/faraday/params_encoders/[nested, flat]
?Faraday v3.0
Faraday::Utils
Faraday::Connection
=>Faraday::Client
#response
property on HTTP-related error classes (RaiseError, RetriableRequest, etc) [Can not view details of failed requests #1284]Faraday::Connection
andFaraday::RackBuilder
relationshipuse
/adapter
/etc delegationFaraday::RackBuilder#handlers
=>Faraday::Connection#handlers
lib/faraday/middleware/*
HTTP/Socks proxy support(needs to be implemented in the http libs themselves)The text was updated successfully, but these errors were encountered: