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

Remove HTTP client creation #110

Merged
merged 3 commits into from
Nov 13, 2023
Merged

Remove HTTP client creation #110

merged 3 commits into from
Nov 13, 2023

Conversation

Jake-Shadle
Copy link
Owner

After #109 and #104 (and #50) I've decided to change the structure of Ctx to force users to pass in their own ureq::Agent that is configured how they want it to be, rather than adding more and more customization into the library itself. xwin was never intended as a library, but since people are using it that way, I don't think it's a good idea to have the HTTP client customization in the library, especially since it's reading environment variables which IMO is not something libraries should do in most cases. This is a bit unfortunate since it means callers have to have their own code if they want to customize, but it means this crate is simpler, and caller's are free to make whatever decisions they want (other than HTTP client, but that's something that can be done later).

@Jake-Shadle
Copy link
Owner Author

Pinging @Owen-CH-Leung in case you have any comments on this. IMO moving the custom cert code to cargo-xwin will be cleaner and (I believe) removes xwin as another step of indirection in the chain of maturin <-> cargo-xwin <-> xwin

@Owen-CH-Leung
Copy link
Contributor

@messense FYI. Feel free to voice your comment here too

@MarijnS95
Copy link
Contributor

I would wait a little bit but it might be useful to use the http crate for client-agnostic requests and responses. The ureq crate had some support for it but it was rather sloppy/inconvenient to use, but that is all cleared up in their main branch and should make it into a release. Now converting between the crates is just one or two lines as it is supposed to be.

@messense
Copy link
Contributor

Looks good to me.

@Jake-Shadle Jake-Shadle merged commit 7b27c96 into main Nov 13, 2023
8 checks passed
@Jake-Shadle Jake-Shadle deleted the pass-in-agent branch November 13, 2023 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants