Skip to content

No Clone for HttpClient-s #48

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

Merged
merged 2 commits into from
Sep 25, 2020
Merged

No Clone for HttpClient-s #48

merged 2 commits into from
Sep 25, 2020

Conversation

Fishrock123
Copy link
Member

@Fishrock123 Fishrock123 commented Sep 24, 2020

(Based on #45 for future simplicity.)

An alternate to #47 "src: require Clone for HttpClient", this PR removes Clone impls from all HttpClients.

This is an alternate to the bottom part of http-rs/surf#237 / #46

The idea is to avoid multiple Arcs, and this pushes the responsibility for sharing a client to the upper client, i.e. Surf. Surf already has to have a pointer for every client anyways, so as to hide the generic from its Client definition.

@Fishrock123 Fishrock123 marked this pull request as draft September 24, 2020 18:50
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Sep 24, 2020
@Fishrock123 Fishrock123 marked this pull request as ready for review September 24, 2020 22:49
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Sep 24, 2020
All of the global shared client now uses just one Arc.

Refs: http-rs/http-client#48
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Sep 24, 2020
Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. It's unfortunate Clone isn't object safe, but this direction seems reasonable. Small note on private constructors, but otherwise looks good!

@Fishrock123 Fishrock123 merged commit 6409e37 into http-rs:main Sep 25, 2020
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Sep 25, 2020
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Sep 25, 2020
All of the global shared client now uses just one Arc.

Refs: http-rs/http-client#48
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Sep 25, 2020
All of the global shared client now uses just one Arc.

Refs: http-rs/http-client#48
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Jul 5, 2021
Kinda reverts http-rs#48

Related to http-rs/surf#237

Desirable for Surf-level config.
@Fishrock123 Fishrock123 deleted the no-clone branch July 5, 2021 17:49
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.

2 participants