Add sequential migration tests for MariaDB and MySQL #2502
Conversation
strickvl
added
internal
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe recent updates involve enhancing two scripts responsible for testing database migrations with a new function called Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
@AlexejPenner as previously discussed, this was one of the low-hanging fruit I said we'd implement on OSS side to better approximate what a user might do. |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- scripts/test-migrations-mariadb.sh (1 hunks)
- scripts/test-migrations-mysql.sh (1 hunks)
Additional comments: 2
scripts/test-migrations-mariadb.sh (1)
- 121-148: The implementation of testing for sequential migrations is a significant enhancement. However, there are a few areas that could be refined:
- The script selects a fixed number of versions (3) for sequential migrations. It might be more flexible to allow the number of versions to be configurable via a script argument or environment variable.
- The sorting of versions using
sortis a clever use of shell utilities. However, consider adding a comment explaining the sorting flags for clarity, as the-t.and-koptions might not be immediately clear to all readers.- The cleanup steps (
zenml disconnect,docker rm -f mariadb,deactivate) are crucial for ensuring that resources are properly released after the tests. It's good practice to also trap errors and ensure these cleanup steps are executed even if the script exits prematurely due to an error.Consider making the number of versions for sequential migrations configurable and adding explanatory comments for the sorting command.
scripts/test-migrations-mysql.sh (1)
- 160-187: The implementation of testing for sequential migrations in the MySQL script, like in the MariaDB script, could benefit from a few refinements:
- Make the number of versions for sequential migrations configurable.
- Add explanatory comments for the sorting command to improve readability.
- Ensure cleanup steps are executed even if the script exits prematurely due to an error by using error trapping.
Consider applying the same improvements suggested for the MariaDB script to enhance flexibility, clarity, and robustness.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai review |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- scripts/test-migrations-mariadb.sh (1 hunks)
- scripts/test-migrations-mysql.sh (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- scripts/test-migrations-mariadb.sh
- scripts/test-migrations-mysql.sh
|
E2E template updates in |
…com:zenml-io/zenml into feature/drunk-hop-skip-db-migration-testing
* Add sequential migration tests for MariaDB and MySQL * add semantic version order * Update scripts/test-migrations-mariadb.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * coderabbit suggestion for version comparison * fresh start for sequential testing * add missing code * Remove unnecessary code in test-migrations-mariadb.sh * list the drunk migration sequence * focused testing * add padding for logs * always install importlib_metadata * fix mysql db upgrades * ruff * fix mariadb too * add mariadb for testing * Auto-update of E2E template * fix mariadb * fresh mariadb * fresh mysql * fix final missing zenml * CI as it should be --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: GitHub Actions <[email protected]>
This pull request adds sequential migration tests for MariaDB and MySQL. The tests ensure that sequential migrations across multiple versions are working correctly.
Our current migration tests go through all versions incrementally, so this adds a final set of tests which picks 3 random versions and then orders them sequentially so that we have something a bit more 'real world' that approximates what a user might do. (DB is reset/wiped in between the first set of tests and the sequential testing.)
Summary by CodeRabbit