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

migration from httpi to faraday #992

Open
pcai opened this issue Mar 12, 2023 · 15 comments
Open

migration from httpi to faraday #992

pcai opened this issue Mar 12, 2023 · 15 comments
Assignees

Comments

@pcai
Copy link
Member

pcai commented Mar 12, 2023

Feature request

httpi is now deprecated in favor of https://github.com/lostisland/faraday, so we should plan a migration to it for savon

Motivation for this feature:

httpi is no longer actively maintained, and continuing to do so would duplicate a lot of existing work already done in the ruby ecosystem.

in addition, there are many missing features in httpi that would require fairly large rewrites (for example -- keepalive support, threadsafety / global state issues come to mind). this effort is better spent migrating savon to faraday which already supports these features.

Would this introduce a breaking change?

yes, this will likely require a major version bump as the public interface for savon, and some important behaviors, might change.

Are you willing to work on this yourself?

yes

@pcai
Copy link
Member Author

pcai commented Mar 27, 2023

I took a look at this recently and it seems to require some larger refactoring changes. Basically, the way the builder pattern is applied needs to change a bit, because faraday distinguishes between "requests" and "connections" much more strongly due to concepts like http keepalive

@pcai pcai self-assigned this Mar 27, 2023
@nowakoo
Copy link

nowakoo commented Apr 27, 2023

@pcai fingers crossed! Do you know approximately when the next version will be released?

@pcai
Copy link
Member Author

pcai commented Apr 28, 2023

Sorry no eta at the moment, I am still poking at this occasionally and there seem to be a bunch of decisions around larger/cleaner refactoring vs minimizing changes. Will update when i make more progress

@bmbliss
Copy link

bmbliss commented Dec 11, 2023

any update on this?

@LukeIGS LukeIGS mentioned this issue Jan 4, 2024
@LukeIGS
Copy link
Contributor

LukeIGS commented Jan 8, 2024

@bmbliss #998

@pcai
Copy link
Member Author

pcai commented Jul 8, 2024

We're getting close to a release candidate with faraday thanks to @LukeIGS (which will eventually be released as savon 3.x).

While I still believe that we should minimize supported versions, for practical reasons I believe we should continue to support 2.x so that httpi-based savon can at least receive security updates for a little while.

@LukeIGS
Copy link
Contributor

LukeIGS commented Jul 8, 2024 via email

@pcai
Copy link
Member Author

pcai commented Jul 8, 2024

I think we are saying the same thing - we will support both major versions of savon simultaneously for some time. This is an exception from the usual policy, which is that there is only 1 supported version at a time (e.g., fixes are never backported to older branch releases).

@pcai
Copy link
Member Author

pcai commented Jul 15, 2024

Released as v3.0.0.rc1 so it can bake a little bit. For anyone who can give this a spin - you can contribute by filing bugs in this issue tracker, or PRs against the upgrade guide

@ehutzelman
Copy link
Contributor

I've upgraded and running on v3.0.0.rc1 successfully so far. Thank you @pcai and @LukeIGS for your work on this!

@pcai
Copy link
Member Author

pcai commented Jul 23, 2024

Hi @ehutzelman thanks so much for reporting back! Did you have to make any code changes other than what was documented in the upgrading guide?

@pcai
Copy link
Member Author

pcai commented Jul 23, 2024

Nevermind - you were the one who made the recent pr so that probably answers my question

@LukeIGS
Copy link
Contributor

LukeIGS commented Jul 23, 2024

So far it appears that I didn't consider the fact that ntlm auth needs to propagate down to wasabi to pull the wsdl. Almost makes me want to strip the concept of an http client out of wasabi entirely and have it take a block from the consumer.

@pcai
Copy link
Member Author

pcai commented Jul 23, 2024

Well, yes, that would be a clear win from a separation-of-concerns point of view.

@Greg-Rose
Copy link

I did a very small amount of testing on v3.0.0.rc1 with the typhoeus adapter in an app that has a pretty simple use case for savon. Just installed the prerelease version and added adapter: :typhoeus to the initialization of the savon client. It worked perfectly!

I also compared the requests before and after, using Jaeger. It seems like persistent connections are now working due to faraday/typhoeus because subsequent requests are now multiple times faster than they used to be. 🎉

Thanks for everyone's work on this! I'm definitely looking forward to the full release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants