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

Update migration command to enable/disable transactions #828

Merged
merged 2 commits into from Jan 22, 2024

Conversation

mmabrouk
Copy link
Contributor

This is a modified version of Beanie that includes an option to enable or disable transactions during migrations. Please see the motivation here (#827). To run a migration without a transaction, use the following command:

beanie migrate --no-use-transaction -uri 'mongodb://username:password@localhost' -db 'db' -p .

Please let me know if you'd like this feature in the upstream, and whether you require any changes to the PR. I will ensure to squash the commits and tidy things up.

@roman-right
Copy link
Owner

Hi @mmabrouk ,
Thank you for the PR! It is useful.
Could you please add a test for this? To guarantee that everything works as expected after future changes.

removed logging

format

added test

Added --no-use-transaction option for migration
@mmabrouk mmabrouk changed the title [WP] Update migration command to enable/disable transactions Update migration command to enable/disable transactions Jan 11, 2024
@mmabrouk mmabrouk marked this pull request as ready for review January 11, 2024 15:40
@mmabrouk
Copy link
Contributor Author

@roman-right I added a simple test. The perfect way to do it would be to add a separate mongodb service (through an action) without replica. But honestly, that would be an overkill.

@mmabrouk
Copy link
Contributor Author

@roman-right Do you have feedback on this?

@roman-right
Copy link
Owner

Hi @mmabrouk ,
Thank you for the provided test suite. Yes, this is enough. I'll double check everything this week and will merge then. Thank you for the work!

@roman-right roman-right merged commit 8fa4068 into roman-right:main Jan 22, 2024
21 checks passed
@roman-right
Copy link
Owner

Hi @mmabrouk ,

Thank you for your work! Merged. It will be published in the next 2 days

@roman-right roman-right mentioned this pull request Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants