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

Impl Clone on DatabaseConnection #293

Closed
robot-head opened this issue Nov 5, 2021 · 3 comments
Closed

Impl Clone on DatabaseConnection #293

robot-head opened this issue Nov 5, 2021 · 3 comments

Comments

@robot-head
Copy link

I saw in #152 that DatabaseConnection was not marked Clone if mocks are enabled, but I see now that the Mock connection variant is behind an Arc.

Is there any reason not to make DatbaseConnection Clone? We've stuffed it into an Arc in our application code, but my code reviewers are questioning why the Arc is necessary.

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 6, 2021

Yes he is right. Do not stuff an Arc in your app code.

Add a feature in your Cargo, and only enable Mock while in testing.

The reason behind that is Mock has a mock output buffer and a transaction log, where it will be hard to reason it's behaviour when being cloned and used across multiple threads.

@robot-head
Copy link
Author

Thanks!

I might send ya a PR on the testing docs to help others that come across this

@PoOnesNerfect
Copy link

It seems like DatabaseConnection::MockDatabaseConnection has inner mock connection struct in an Arc, making it cloneable.
It seems strange that all variants of this DatabaseConnection derives Clone including Arc<MockDatabaseConnection>, but the wrapper enum itself does not implement Clone when mock feature is enabled.

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

No branches or pull requests

3 participants