-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
✨ add MySQL 8.4 and MariaDB 11.4 support #85
Conversation
WalkthroughThe recent updates enhance the CI/CD pipeline by broadening support for various Python and MySQL versions, improving character set and collation handling, and refining the command-line interface for migrations. These changes improve compatibility and flexibility while streamlining database setup and user creation processes, ensuring better functionality and security. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant MigrationTool
participant MySQL
participant SQLite
User->>CLI: Run migration command with charset and collation
CLI->>MigrationTool: Pass charset and collation
MigrationTool->>MySQL: Connect with specified charset and collation
MigrationTool->>SQLite: Transfer data
MySQL->>MigrationTool: Provide data
MigrationTool->>SQLite: Store data
SQLite->>User: Migration complete
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 Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #85 +/- ##
==========================================
- Coverage 87.16% 85.85% -1.32%
==========================================
Files 8 8
Lines 592 629 +37
==========================================
+ Hits 516 540 +24
- Misses 76 89 +13 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- .github/workflows/test.yml (3 hunks)
- README.md (1 hunks)
- pyproject.toml (1 hunks)
- requirements_dev.txt (1 hunks)
- src/mysql_to_sqlite3/cli.py (4 hunks)
- src/mysql_to_sqlite3/mysql_utils.py (1 hunks)
- src/mysql_to_sqlite3/transporter.py (3 hunks)
- src/mysql_to_sqlite3/types.py (2 hunks)
- tests/conftest.py (1 hunks)
- tests/func/mysql_to_sqlite3_test.py (3 hunks)
- tests/func/test_cli.py (6 hunks)
Additional context used
GitHub Check: codecov/patch
src/mysql_to_sqlite3/mysql_utils.py
[warning] 27-33: src/mysql_to_sqlite3/mysql_utils.py#L27-L33
Added lines #L27 - L33 were not covered by tests
[warning] 40-41: src/mysql_to_sqlite3/mysql_utils.py#L40-L41
Added lines #L40 - L41 were not covered by testssrc/mysql_to_sqlite3/transporter.py
[warning] 70-70: src/mysql_to_sqlite3/transporter.py#L70
Added line #L70 was not covered by tests
LanguageTool
README.md
[locale-violation] ~5-~5: license must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...+10.3+|+10.4+|+10.5|+11.4&color=C0765A) [![GitHub license](https://img.shields.io/github/license/...(LICENCE_LICENSE_NOUN_SINGULAR)
Additional comments not posted (29)
requirements_dev.txt (1)
5-5
: Approve the version update but verify compatibility.The update to
mysql-connector-python
version 9.0.0 may include enhancements, bug fixes, or breaking changes. Ensure that the existing codebase is compatible with this new version.src/mysql_to_sqlite3/mysql_utils.py (2)
5-5
: Approve the import ofCharacterSet
andMYSQL_CHARACTER_SETS
.The imports are necessary for handling MySQL character sets.
14-20
: Approve theCharSet
named tuple.The
CharSet
named tuple provides a clear and structured representation of MySQL character sets.src/mysql_to_sqlite3/types.py (2)
27-28
: Approve the addition ofmysql_charset
andmysql_collation
toMySQLtoSQLiteParams
.The new attributes enhance the capability to specify character set and collation settings for MySQL connections.
60-61
: Approve the addition of_mysql_charset
and_mysql_collation
toMySQLtoSQLiteAttributes
.The new attributes enhance the capability to manage character set and collation settings for MySQL connections.
Verification successful
Approve the addition of
_mysql_charset
and_mysql_collation
toMySQLtoSQLiteAttributes
.The attributes are properly integrated and used within the codebase, enhancing the capability to manage character set and collation settings for MySQL connections.
src/mysql_to_sqlite3/types.py
src/mysql_to_sqlite3/transporter.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new attributes _mysql_charset and _mysql_collation. # Test: Search for the usage of the new attributes. Expect: Proper usage in the codebase. rg --type python '_mysql_charset' rg --type python '_mysql_collation'Length of output: 131
Script:
#!/bin/bash # Description: Verify the usage of new attributes _mysql_charset and _mysql_collation. # Test: Search for the usage of the new attributes. Expect: Proper usage in the codebase. rg '_mysql_charset' --glob '*.py' rg '_mysql_collation' --glob '*.py'Length of output: 908
pyproject.toml (1)
42-42
: Ensure compatibility with the newmysql-connector-python
version.The version constraint for
mysql-connector-python
has been updated from8.4.0
to>=9.0.0
. Verify that the new version does not introduce breaking changes and is compatible with the rest of the codebase.README.md (2)
4-4
: Update the MySQL support badge.The MySQL support badge has been updated to include version 8.4, enhancing clarity for users.
5-5
: Update the MariaDB support badge.The MariaDB support badge has been updated to include version 11.4, enhancing clarity for users.
Tools
LanguageTool
[locale-violation] ~5-~5: license must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...+10.3+|+10.4+|+10.5|+11.4&color=C0765A) [![GitHub license](https://img.shields.io/github/license/...(LICENCE_LICENSE_NOUN_SINGULAR)
src/mysql_to_sqlite3/cli.py (6)
9-9
: New import:CharacterSet
frommysql.connector
.Ensure that the
CharacterSet
class is correctly used in the new CLI options.
16-16
: New import:mysql_supported_character_sets
frommysql_utils
.Ensure that the
mysql_supported_character_sets
function is correctly used in the new CLI options.
112-118
: Add new CLI option:--mysql-charset
.The new option allows users to specify the MySQL character set. Ensure it is correctly integrated and validated.
120-128
: Add new CLI option:--mysql-collation
.The new option allows users to specify the MySQL collation. Ensure it is correctly integrated and validated.
172-173
: Update function signature:cli
to includemysql_charset
andmysql_collation
.The function signature has been updated to include the new parameters. Ensure that these values are correctly passed and utilised in the migration process.
210-211
: Pass new parameters toMySQLtoSQLite
constructor.The new parameters
mysql_charset
andmysql_collation
are passed to theMySQLtoSQLite
constructor. Ensure that these parameters are correctly used within the class.tests/conftest.py (1)
250-251
: LGTM! But verify the function usage in the codebase.The addition of
charset
andcollation
parameters improves the function's ability to handle diverse character sets. The code changes are approved.However, ensure that all function calls to
mysql_instance
match the new signature.Verification successful
Verified: No further changes needed.
The
mysql_instance
function is used as a fixture, and the new parameters (charset
andcollation
) are set within the fixture definition itself. There are no direct calls tomysql_instance
that require these parameters to be explicitly passed.
tests/conftest.py
: Fixture definition updated with new parameters.tests/unit/mysql_to_sqlite3_test.py
: Usesmysql_instance
as a fixture.tests/func/test_cli.py
: Usesmysql_instance
as a fixture.tests/func/mysql_to_sqlite3_test.py
: Usesmysql_instance
as a fixture.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `mysql_instance` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'mysql_instance'Length of output: 69
Script:
#!/bin/bash # Description: Verify all function calls to `mysql_instance` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg -A 5 'mysql_instance'Length of output: 1953
Script:
#!/bin/bash # Description: Extract more context around the lines where `mysql_instance` is used to verify the function calls match the new signature. # Extracting context around the lines where `mysql_instance` is used in `tests/conftest.py` rg 'mysql_instance' -A 10 -B 10 tests/conftest.py # Extracting context around the lines where `mysql_instance` is used in `tests/unit/mysql_to_sqlite3_test.py` rg 'mysql_instance' -A 10 -B 10 tests/unit/mysql_to_sqlite3_test.py # Extracting context around the lines where `mysql_instance` is used in `tests/func/test_cli.py` rg 'mysql_instance' -A 10 -B 10 tests/func/test_cli.py # Extracting context around the lines where `mysql_instance` is used in `tests/func/mysql_to_sqlite3_test.py` rg 'mysql_instance' -A 10 -B 10 tests/func/mysql_to_sqlite3_test.pyLength of output: 3870
.github/workflows/test.yml (3)
309-337
: Approved: Added matrix configurations for MariaDB 11.4.The new entries ensure compatibility with MariaDB 11.4 across multiple Python versions.
459-487
: Approved: Added matrix configurations for MySQL 8.4.The new entries ensure compatibility with MySQL 8.4 across multiple Python versions, while marking them as experimental due to known issues.
526-565
: Approved: Enhanced MySQL setup steps.The changes handle specific MySQL versions (8.0 and 8.4) and manage certificate files, improving the security and functionality of the database setup process.
tests/func/test_cli.py (6)
89-92
: Approved: Enhanced test for invalid database name.The test now includes host and port options, covering more scenarios and clarifying its intent by marking it as expected to fail.
116-119
: Approved: Enhanced test for invalid database user.The test now includes host and port options, covering more scenarios and clarifying its intent by marking it as expected to fail.
145-148
: Approved: Enhanced test for invalid database password.The test now includes host and port options, covering more scenarios and clarifying its intent by marking it as expected to fail.
171-174
: Approved: Enhanced test for database password prompt.The test now includes host and port options, covering more scenarios.
199-202
: Approved: Enhanced test for invalid database password prompt.The test now includes host and port options, covering more scenarios and clarifying its intent by marking it as expected to fail.
209-212
: Approved: Enhanced test for invalid database port.The test now includes host and port options, covering more scenarios and clarifying its intent by marking it as expected to fail.
src/mysql_to_sqlite3/transporter.py (2)
64-71
: Ensure charset and collation handling is correct.The charset and collation handling appears correct. However, ensure that the charset and collation values are valid and supported by the MySQL version in use.
Verification successful
Charset and collation handling is correct.
The charset
utf8mb4
and collationsutf8mb4_unicode_ci
andutf8mb4_0900_ai_ci
are used consistently across the codebase, including in tests, indicating they are valid and supported by MySQL.
tests/conftest.py
src/mysql_to_sqlite3/transporter.py
src/mysql_to_sqlite3/cli.py
tests/unit/mysql_to_sqlite3_test.py
tests/func/mysql_to_sqlite3_test.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify charset and collation values are valid and supported by MySQL. # Test: Search for charset and collation values in the MySQL documentation or source code. rg --type markdown 'utf8mb4|utf8mb4_unicode_ci|utf8mb4_0900_ai_ci' docs/Length of output: 74
Script:
#!/bin/bash # Description: Verify charset and collation values are valid and supported by MySQL. # Test: Search for charset and collation values in the entire codebase. rg 'utf8mb4|utf8mb4_unicode_ci|utf8mb4_0900_ai_ci'Length of output: 1166
Tools
GitHub Check: codecov/patch
[warning] 70-70: src/mysql_to_sqlite3/transporter.py#L70
Added line #L70 was not covered by tests
139-140
: Verify charset and collation parameters in MySQL connection.Ensure that the charset and collation parameters are correctly passed and handled by the MySQL connection. Additionally, verify that the MySQL server supports these parameters.
Verification successful
Verification successful: Charset and collation parameters are correctly passed and handled in the MySQL connection setup.
The parameters
charset
andcollation
are used appropriately in the MySQL connection setup and related utility functions. They are also defined in type hints and used in CLI options, ensuring comprehensive handling.
src/mysql_to_sqlite3/transporter.py
: Lines 139-140src/mysql_to_sqlite3/types.py
src/mysql_to_sqlite3/mysql_utils.py
src/mysql_to_sqlite3/cli.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify charset and collation parameters in MySQL connection. # Test: Search for the usage of charset and collation parameters in the MySQL connection code. rg --type python 'charset|collation' src/Length of output: 74
Script:
#!/bin/bash # Description: Verify charset and collation parameters in MySQL connection. # Test: Search for the usage of charset and collation parameters in the MySQL connection code. rg --type py 'charset|collation' src/Length of output: 5007
tests/func/mysql_to_sqlite3_test.py (3)
72-72
: Verify the use of@pytest.mark.xfail
.The
@pytest.mark.xfail
decorator is used to mark this test as expected to fail. Ensure that this is intentional and correctly implemented.Verification successful
Verified the use of
@pytest.mark.xfail
.The
@pytest.mark.xfail
decorator is used consistently across the test suite to mark tests expected to fail. This usage appears intentional and correctly implemented.
tests/func/mysql_to_sqlite3_test.py
tests/func/test_cli.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the use of `@pytest.mark.xfail` in the test suite. # Test: Search for the usage of `@pytest.mark.xfail` in the test suite. rg --type python '@pytest.mark.xfail' tests/Length of output: 75
Script:
#!/bin/bash # Description: Verify the use of `@pytest.mark.xfail` in the test suite. # Test: Search for the usage of `@pytest.mark.xfail` in the test suite. rg '@pytest.mark.xfail' --glob '*.py' tests/Length of output: 438
467-468
: Ensure charset and collation parameters are correctly tested.The charset and collation parameters are added to the test function. Ensure that these parameters are correctly used and tested.
1217-1218
: Ensure charset and collation parameters are correctly tested.The charset and collation parameters are added to the test function. Ensure that these parameters are correctly used and tested.
def mysql_supported_character_sets(charset: t.Optional[str] = None) -> t.Iterator[CharSet]: | ||
"""Get supported MySQL character sets.""" | ||
index: int | ||
info: t.Optional[t.Tuple[str, str, bool]] | ||
if charset is not None: | ||
for index, info in enumerate(MYSQL_CHARACTER_SETS): | ||
if info is not None: | ||
try: | ||
if info[0] == charset: | ||
yield CharSet(index, charset, info[1]) | ||
except KeyError: | ||
continue | ||
else: | ||
for charset in CharacterSet().get_supported(): | ||
for index, info in enumerate(MYSQL_CHARACTER_SETS): | ||
if info is not None: | ||
try: | ||
yield CharSet(index, charset, info[1]) | ||
except KeyError: | ||
continue |
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.
Approve the mysql_supported_character_sets
function but add tests for uncovered lines.
The function enhances the module's capability to handle character sets. However, the lines 27-33 and 40-41 are not covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 27-33: src/mysql_to_sqlite3/mysql_utils.py#L27-L33
Added lines #L27 - L33 were not covered by tests
[warning] 40-41: src/mysql_to_sqlite3/mysql_utils.py#L40-L41
Added lines #L40 - L41 were not covered by tests
kwargs.get("mysql_collation") or CharacterSet().get_default_collation(self._mysql_charset.lower())[0] | ||
) | ||
if not kwargs.get("mysql_collation") and self._mysql_collation == "utf8mb4_0900_ai_ci": | ||
self._mysql_collation = "utf8mb4_unicode_ci" |
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.
Add test coverage for charset and collation handling.
The new functionality for charset and collation handling is not covered by tests. Ensure that tests are added to cover these changes.
Do you want me to generate the test code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 70-70: src/mysql_to_sqlite3/transporter.py#L70
Added line #L70 was not covered by tests
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/mysql_to_sqlite3/cli.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/mysql_to_sqlite3/cli.py
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- README.md (3 hunks)
- docs/README.rst (1 hunks)
Additional context used
LanguageTool
README.md
[locale-violation] ~5-~5: license must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...+10.3+|+10.4+|+10.5|+11.4&color=C0765A) [![GitHub license](https://img.shields.io/github/license/...(LICENCE_LICENSE_NOUN_SINGULAR)
Additional comments not posted (4)
docs/README.rst (2)
47-47
: Documentation Update: Addition of--mysql-charset TEXT
OptionThe new
--mysql-charset TEXT
option is well-documented, specifying that it sets the character set for the MySQL database and tables, with a default value ofutf8mb4
.
48-48
: Documentation Update: Addition of--mysql-collation TEXT
OptionThe new
--mysql-collation TEXT
option is well-documented, allowing users to set the collation for the MySQL database and tables.README.md (2)
4-4
: Update: MySQL Support BadgeThe MySQL support badge has been correctly updated to include version 8.4.
5-5
: Update: MariaDB Support BadgeThe MariaDB support badge has been correctly updated to include version 11.4.
Tools
LanguageTool
[locale-violation] ~5-~5: license must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...+10.3+|+10.4+|+10.5|+11.4&color=C0765A) [![GitHub license](https://img.shields.io/github/license/...(LICENCE_LICENSE_NOUN_SINGULAR)
Pull Request Template
Description
Add MariaDB 11.4 and MySQL 8.4 support
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Additional MariaDB 11.4 and MySQL 8.4 specific CI tests have been added.
NOTE: Because of a MySQL 8.4 bug all MySQL 8.4 tests have been marked as experimental which effectively sets
continue-on-error: true
for these tests.Checklist: