✨ exclude transferring SQLite tables via -e/--exclude-sqlite-tables
#146
Conversation
WalkthroughAdds an exclude-list option for SQLite tables across CLI, transporter, and types; enforces mutual exclusivity with the existing include list; updates sqlite_master selection to use IN or NOT IN; and extends functional and CLI tests to cover include vs exclude behaviour and the exclusivity check. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (sqlite3mysql)
participant Transporter as SQLite3toMySQL
participant SQLite as SQLite DB
participant MySQL as MySQL DB
User->>CLI: Run with args (-t include | -e exclude)
CLI->>CLI: Validate mutual exclusivity
alt Both provided
CLI-->>User: Error (ClickException)
else One provided
CLI->>Transporter: Instantiate (sqlite_tables or exclude_sqlite_tables)
Transporter->>Transporter: Validate exclusivity & set without_foreign_keys
Transporter->>SQLite: Query sqlite_master with IN / NOT IN (based on mode)
SQLite-->>Transporter: Table list
loop For each selected table
Transporter->>MySQL: Create schema/table/index
Transporter->>SQLite: Read rows
Transporter->>MySQL: Insert rows
end
Transporter-->>CLI: Completion
CLI-->>User: "Done!"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
techouse
changed the title
-e/--exclude-sqlite-tables-e/--exclude-sqlite-tables
techouse
changed the title
-e/--exclude-sqlite-tables-e/--exclude-sqlite-tables
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/sqlite3_to_mysql/transporter.py (1)
868-885: Consider parameterised query safety patterns.Whilst the current implementation uses parameterised queries correctly with placeholders and bound parameters, the string formatting for the
NOT/INclause construction could be made more explicit about its safety. Consider adding a comment to clarify thatexcludeandplaceholdersare safely constructed and not user-controlled.self._sqlite_cur.execute( """ SELECT name FROM sqlite_master WHERE type='table' AND name NOT LIKE 'sqlite_%' + -- Safe: 'exclude' is either "NOT" or "", placeholders are '?' repeated AND name {exclude} IN ({placeholders}) """.format( exclude="NOT" if len(self._exclude_sqlite_tables) > 0 else "", placeholders=", ".join("?" * len(specific_tables)), ), specific_tables, )src/sqlite3_to_mysql/cli.py (1)
204-205: Consider updating the foreign keys logic.The
without_foreign_keysparameter calculation on line 205 only checkssqlite_tablesbut doesn't account forexclude_sqlite_tables. However, this appears to be handled correctly in the transporter's__init__method. Consider simplifying this line for clarity:sqlite_tables=sqlite_tables or tuple(), exclude_sqlite_tables=exclude_sqlite_tables or tuple(), - without_foreign_keys=without_foreign_keys or (sqlite_tables is not None and len(sqlite_tables) > 0), + without_foreign_keys=without_foreign_keys,The transporter already handles this logic internally, so passing the raw value would be cleaner and avoid duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/sqlite3_to_mysql/cli.py(3 hunks)src/sqlite3_to_mysql/transporter.py(2 hunks)src/sqlite3_to_mysql/types.py(2 hunks)tests/func/sqlite3_to_mysql_test.py(7 hunks)tests/func/test_cli.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/sqlite3_to_mysql/cli.py (1)
src/sqlite3_to_mysql/click_utils.py (1)
OptionEatAll(8-54)
tests/func/sqlite3_to_mysql_test.py (1)
tests/conftest.py (1)
sqlite_database(150-187)
tests/func/test_cli.py (1)
tests/conftest.py (4)
cli_runner(333-334)sqlite_database(150-187)mysql_credentials(206-232)MySQLCredentials(195-202)
src/sqlite3_to_mysql/transporter.py (1)
tests/unit/sqlite3_to_mysql_test.py (8)
execute(248-249)execute(281-282)execute(364-365)execute(418-419)execute(460-465)execute(519-520)execute(565-566)execute(615-621)
🪛 Ruff (0.13.1)
src/sqlite3_to_mysql/cli.py
196-199: Abstract raise to an inner function
(TRY301)
196-199: Avoid specifying long messages outside the exception class
(TRY003)
src/sqlite3_to_mysql/transporter.py
115-115: Avoid specifying long messages outside the exception class
(TRY003)
875-883: Possible SQL injection vector through string-based query construction
(S608)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
src/sqlite3_to_mysql/types.py (2)
25-25: LGTM! Clean addition of the exclusion parameter.The new
exclude_sqlite_tablesparameter is properly typed asOptional[Sequence[str]], maintaining consistency with the existingsqlite_tablesparameter.
56-56: LGTM! Internal attribute properly aligned.The
_exclude_sqlite_tablesattribute correctly mirrors the public parameter type asSequence[str], consistent with_sqlite_tables.src/sqlite3_to_mysql/transporter.py (1)
112-121: LGTM! Robust mutual exclusivity validation.The implementation correctly:
- Initialises
_exclude_sqlite_tableswith a default empty tuple- Validates mutual exclusivity between include and exclude options
- Appropriately sets
_without_foreign_keyswhen either option is providedsrc/sqlite3_to_mysql/cli.py (2)
44-52: LGTM! Well-documented CLI option.The new
--exclude-sqlite-tablesoption is properly implemented with:
- Clear help text explaining the mutual exclusivity with
--sqlite-tables- Appropriate use of
OptionEatAllfor space-separated table names- Consistent parameter structure with the existing include option
195-199: LGTM! Clear error message for mutual exclusivity.The validation correctly enforces that users cannot specify both include and exclude options simultaneously, with a helpful error message.
tests/func/sqlite3_to_mysql_test.py (3)
461-525: LGTM! Comprehensive test coverage for exclusion feature.The parameterised test thoroughly covers all combinations of:
- Chunk/no chunk
- With/without rowid
- All insert methods (IGNORE, UPDATE, DEFAULT)
- Duplicate key handling
- Both include and exclude modes
541-543: LGTM! Clean calculation of remaining tables.The logic correctly computes the set difference to determine which tables should be transferred when using exclusion mode.
560-569: LGTM! Proper assertion of log messages.The test correctly verifies that:
- The appropriate tables are logged as being transferred (remaining when excluding, random when including)
- The "Done!" message is logged upon completion
tests/func/test_cli.py (1)
632-665: Incorrect — duplicate not foundRepository search shows a single occurrence at tests/func/test_cli.py:632; no duplicate exists and the test should not be removed.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/sqlite3_to_mysql/cli.py (3)
44-52: Fix help-string grammar; mirror exclusivity on -t optionChange “Can not” → “Cannot”. Also consider adding the same exclusivity note to the -t/--sqlite-tables help for symmetry.
- "Can not be used together with --sqlite-tables.", + "Cannot be used together with --sqlite-tables.",Additionally (outside this hunk), consider updating the -t/--sqlite-tables help to also state:
"""
Cannot be used together with --exclude-sqlite-tables.
"""
195-200: Tighten mutual‑exclusion check to detect “provided” vs “populated”Prefer explicit None checks to make intent clear and robust even if an empty tuple is ever passed.
- if exclude_sqlite_tables and sqlite_tables: + if exclude_sqlite_tables is not None and sqlite_tables is not None:
204-209: Simplify without_foreign_keys calculationSame behaviour, clearer intent.
- without_foreign_keys=without_foreign_keys - or ( - (sqlite_tables is not None and len(sqlite_tables) > 0) - or (exclude_sqlite_tables is not None and len(exclude_sqlite_tables) > 0) - ), + without_foreign_keys=without_foreign_keys + or bool(sqlite_tables) + or bool(exclude_sqlite_tables),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sqlite3_to_mysql/cli.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sqlite3_to_mysql/cli.py (2)
src/sqlite3_to_mysql/click_utils.py (1)
OptionEatAll(8-54)src/sqlite3_to_mysql/transporter.py (1)
SQLite3toMySQL(58-985)
🪛 Ruff (0.13.1)
src/sqlite3_to_mysql/cli.py
196-199: Abstract raise to an inner function
(TRY301)
196-199: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
src/sqlite3_to_mysql/cli.py (1)
148-148: Signature update LGTMParameter name aligns with the option and downstream usage.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #146 +/- ##
==========================================
- Coverage 93.85% 93.81% -0.05%
==========================================
Files 8 8
Lines 797 808 +11
==========================================
+ Hits 748 758 +10
- Misses 49 50 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/sqlite3_to_mysql/cli.py (3)
44-52: Polish help text and UX for the new option (metavar + wording).
- Add a metavar to show multi‑value usage clearly.
- Prefer “Cannot” in the help string.
Apply this diff:
@click.option( "-e", "--exclude-sqlite-tables", type=tuple, cls=OptionEatAll, + metavar="TABLE [TABLE ...]", help="Transfer all tables except these specific tables (space separated table names). " "Implies --without-foreign-keys which inhibits the transfer of foreign keys. " - "Can not be used together with --sqlite-tables.", + "Cannot be used together with --sqlite-tables.", )
195-200: Prefer a truthiness check and raise UsageError (addresses Ruff TRY003/TRY301).
- Use
if sqlite_tables and exclude_sqlite_tables:to cover empty tuples.- Raise
click.UsageErrorfor CLI misuse.- Optionally, consider moving this (and other pre-run validations) outside the big try block later to narrow exception scope.
Apply this diff:
- if exclude_sqlite_tables is not None and sqlite_tables is not None: - raise click.ClickException( - "Error: Both -t/--sqlite-tables and -e/--exclude-sqlite-tables options are set. " - "Please use only one of them." - ) + if sqlite_tables and exclude_sqlite_tables: + raise click.UsageError( + "Both -t/--sqlite-tables and -e/--exclude-sqlite-tables are set. Please use only one." + )
204-206: Avoid duplicating implied foreign-keys logic; let the transporter enforce it.
transporter.SQLite3toMySQLalready forceswithout_foreign_keys=Truewhen include/exclude lists are set. Passing the derived value here duplicates policy and risks drift.Apply this diff:
sqlite_tables=sqlite_tables or tuple(), exclude_sqlite_tables=exclude_sqlite_tables or tuple(), - without_foreign_keys=without_foreign_keys or bool(sqlite_tables) or bool(exclude_sqlite_tables), + without_foreign_keys=without_foreign_keys,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sqlite3_to_mysql/cli.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sqlite3_to_mysql/cli.py (1)
src/sqlite3_to_mysql/click_utils.py (1)
OptionEatAll(8-54)
🪛 Ruff (0.13.1)
src/sqlite3_to_mysql/cli.py
196-199: Abstract raise to an inner function
(TRY301)
196-199: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
src/sqlite3_to_mysql/cli.py (1)
148-148: Signature update LGTM.Adding
exclude_sqlite_tables: Optional[Sequence[str]]aligns with the new CLI option and downstream usage.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/sqlite3_to_mysql/cli.py (1)
195-200: Mutual‑exclusivity guard is correct; consider click.BadOptionUsage for UX.Current behaviour is fine. Optionally use click.BadOptionUsage to point users at the offending option and align with Click idioms (also satisfies Ruff TRY003/TRY301 hints).
Apply this diff:
- raise click.ClickException( - "Error: Both -t/--sqlite-tables and -e/--exclude-sqlite-tables options are set. " - "Please use only one of them." - ) + raise click.BadOptionUsage( + option_name="-t/--sqlite-tables", + message="Options -t/--sqlite-tables and -e/--exclude-sqlite-tables are mutually exclusive. Use only one.", + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sqlite3_to_mysql/cli.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sqlite3_to_mysql/cli.py (2)
src/sqlite3_to_mysql/click_utils.py (1)
OptionEatAll(8-54)src/sqlite3_to_mysql/transporter.py (1)
SQLite3toMySQL(58-985)
🪛 Ruff (0.13.1)
src/sqlite3_to_mysql/cli.py
196-199: Abstract raise to an inner function
(TRY301)
196-199: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, true, 3.13)
- GitHub Check: Test (python3.10, mariadb:10.0, 1, true, 3.10)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, true, 3.11)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, true, 3.11)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, true, 3.9)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, true, 3.12)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, true, 3.9)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, true, 3.10)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/sqlite3_to_mysql/cli.py (2)
148-148: Signature update wired through — LGTM.The new exclude_sqlite_tables parameter is correctly added and typed.
204-206: Approve — implied --without-foreign-keys with include/exclude is covered.transporter sets _without_foreign_keys when sqlite_tables or exclude_sqlite_tables are provided (src/sqlite3_to_mysql/transporter.py:112–120); CLI enforces mutual exclusivity (src/sqlite3_to_mysql/cli.py:195–199); functional and CLI tests cover include/exclude and the exclusivity error (tests/func/sqlite3_to_mysql_test.py:545–548; tests/func/test_cli.py:660–664).
|
Thanks, works perfectly ! |
This pull request adds support for excluding specific tables from being transferred during a SQLite-to-MySQL migration, in addition to the existing option to include only specific tables. It also ensures that the include and exclude options are mutually exclusive, updates internal logic to support the new feature, and expands test coverage for these scenarios.
New CLI Feature: Table Exclusion
-e/--exclude-sqlite-tablesto transfer all tables except the specified ones. This option cannot be used together with-t/--sqlite-tables, and both options now imply--without-foreign-keys. [1] [2] [3]Core Logic Updates
SQLite3toMySQLclass and related types to handle the newexclude_sqlite_tablesparameter, ensuring only one ofsqlite_tablesorexclude_sqlite_tablescan be set and adjusting foreign key transfer logic accordingly. [1] [2] [3]Testing Enhancements