-
Notifications
You must be signed in to change notification settings - Fork 5
Add ScalarDB Cluster test with MySQL #170
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
Conversation
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds MySQL support as a storage backend for ScalarDB Cluster tests. The implementation refactors how database types are handled, moving from a :cluster database option to using storage backends (postgres/mysql) with an environment variable flag to enable cluster mode.
Key changes:
- Added MySQL as a supported storage backend alongside PostgreSQL for ScalarDB Cluster tests
- Refactored database configuration to use environment variables for Docker credentials and cluster mode detection
- Updated GitHub Actions workflows to use the new configuration approach with storage backend selection
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scalardb/src/scalardb/runner.clj | Renamed db-keys to supported-dbs, added :mysql support, refactored gen-db to route based on :cluster? env var instead of db type, removed docker credential CLI options |
| scalardb/src/scalardb/db_extend.clj | Removed get-db-type method from DbExtension protocol |
| scalardb/src/scalardb/db/postgres.clj | Removed get-db-type implementation, updated gen-db signature to accept variadic args |
| scalardb/src/scalardb/db/cluster.clj | Added MySQL database setup, refactored configuration to use environment variables for Docker credentials, added database-specific connection properties |
| scalardb/src/scalardb/db/cassandra.clj | Updated gen-db signature to accept variadic args |
| scalardb/project.clj | Added :cluster? environment variable flag |
| README.md | Updated documentation to reflect new environment variable approach for Docker credentials |
| .github/workflows/test.yml | Updated to use environment variables and --db postgres instead of --db cluster |
| .github/workflows/daily.yml | Updated to use environment variables and --db postgres instead of --db cluster |
| .github/workflows/cluster-test.yml | Added storage backend selection input, changed Java version to 21, updated to use environment variables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
feeblefakie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looking good. Left one comment. PTAL!
feeblefakie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Left one question for clarification.
brfrn169
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Description
Add a test with MySQL
Related issues and/or PRs
Depends on #169
Changes made
Checklist
Additional notes (optional)