weiznich / diesel_async

Diesel async connection implementation
Apache License 2.0
656 stars 81 forks source link

Add Clone for SyncConnectionWrapper #177

Closed pickfire closed 3 months ago

pickfire commented 3 months ago

Clone is needed for some async stuff like teloxide.

See discussion in https://github.com/teloxide/teloxide/discussions/1124

weiznich commented 3 months ago

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 commented 3 months ago

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