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

Add Clone for SyncConnectionWrapper #177

Closed
wants to merge 1 commit into from
Closed

Conversation

pickfire
Copy link

@pickfire pickfire commented Aug 6, 2024

Clone is needed for some async stuff like teloxide.

See discussion in teloxide/teloxide#1124

Clone is needed for some async stuff like teloxide.
@weiznich
Copy link
Owner

weiznich commented Aug 6, 2024

Thanks for opening this PR.

Unfortunately I don't think it's a great idea to implement Clone for any connection type as you generally do not want to share the connection in that way. Instead prefer using a connection pool. These pool types usually implement Clone (and Sync) which makes them ideal candidates for being part of a service state. It also allows your service to use more than one connection at the same time.

For this particular type there is the additional constraint that some of the internal code (e.g the implementation for the instrumentation function) relies on the fact that the connection cannot be cloned.

@pickfire
Copy link
Author

pickfire commented Aug 8, 2024

Thanks, that seemed to be able to solve the issue.

@pickfire pickfire closed this Aug 8, 2024
@pickfire pickfire deleted the clone branch August 8, 2024 10:50
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