✨ add table creation order computation with FK constraints #98
Conversation
Introduce `compute_creation_order` utility to determine table creation order based on foreign key dependencies. Update transporter to respect this order and log cyclic dependencies. Add handling for foreign key violations post transfer.
Extend unit tests to cover `compute_creation_order`, `fetch_schema_metadata`, and transporter table transfer behavior. Introduce tests for foreign key dependency resolution, circular dependencies, and fallback mechanisms.
WalkthroughNew utilities for extracting MySQL schema metadata and determining table creation order respecting foreign key constraints were introduced. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MySQLtoSQLite
participant MySQLConn
participant SQLiteConn
User->>MySQLtoSQLite: transfer()
MySQLtoSQLite->>MySQLConn: fetch_schema_metadata()
MySQLConn-->>MySQLtoSQLite: tables, FK edges
MySQLtoSQLite->>MySQLtoSQLite: topo_sort_tables()
alt Cyclic dependencies detected
MySQLtoSQLite->>MySQLtoSQLite: Log warning
end
MySQLtoSQLite->>SQLiteConn: PRAGMA foreign_keys=OFF
loop For each table in order
MySQLtoSQLite->>SQLiteConn: Transfer table data
end
MySQLtoSQLite->>SQLiteConn: PRAGMA foreign_keys=ON
MySQLtoSQLite->>SQLiteConn: PRAGMA foreign_key_check
SQLiteConn-->>MySQLtoSQLite: FK violations (if any)
MySQLtoSQLite->>MySQLtoSQLite: Log FK violations
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #98 +/- ##
==========================================
- Coverage 94.34% 94.06% -0.29%
==========================================
Files 8 8
Lines 637 708 +71
==========================================
+ Hits 601 666 +65
- Misses 36 42 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/mysql_to_sqlite3/transporter.py (1)
784-785: Improve type annotation for FK violationsConsider using a more specific type annotation that reflects the actual structure returned by SQLite's
PRAGMA foreign_key_check.- fk_violations: t.List[t.Tuple[t.Any, ...]] = self._sqlite_cur.fetchall() + fk_violations: t.List[sqlite3.Row] = self._sqlite_cur.fetchall()tests/unit/test_transporter.py (5)
241-241: Remove redundant type annotations.The type annotations here are redundant since the variables are immediately assigned
MagicMock()instances.- mock_sqlite_cursor: MagicMock = MagicMock() + mock_sqlite_cursor = MagicMock() ... - mock_sqlite_connection: MagicMock = MagicMock() + mock_sqlite_connection = MagicMock() ... - mock_mysql_connection: MagicMock = MagicMock() + mock_mysql_connection = MagicMock()Also applies to: 244-244, 253-253
286-286: Uset.Listfor Python <3.9 compatibility.The
list[t.Any]syntax requires Python 3.9+. Uset.List[t.Any]for broader compatibility.- creation_calls: list[t.Any] = [call[0][0] for call in instance._create_table.call_args_list] + creation_calls: t.List[t.Any] = [call[0][0] for call in instance._create_table.call_args_list]
304-304: Remove redundant type annotations.Same issue as in the previous test - these type annotations are redundant.
- mock_sqlite_cursor: MagicMock = MagicMock() + mock_sqlite_cursor = MagicMock() ... - mock_sqlite_connection: MagicMock = MagicMock() + mock_sqlite_connection = MagicMock() ... - mock_mysql_connection: MagicMock = MagicMock() + mock_mysql_connection = MagicMock()Also applies to: 307-307, 316-316
354-354: Uset.Listfor Python <3.9 compatibility.Same compatibility issue as in the previous test.
- creation_calls: t.List[t.Any] = [call[0][0] for call in instance._create_table.call_args_list] + creation_calls: t.List[t.Any] = [call[0][0] for call in instance._create_table.call_args_list]
372-372: Remove redundant type annotations.Same redundant type annotation pattern as in the other tests.
- mock_sqlite_cursor: MagicMock = MagicMock() + mock_sqlite_cursor = MagicMock() ... - mock_sqlite_connection: MagicMock = MagicMock() + mock_sqlite_connection = MagicMock() ... - mock_mysql_connection: MagicMock = MagicMock() + mock_mysql_connection = MagicMock()Also applies to: 375-375, 384-384
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/mysql_to_sqlite3/mysql_utils.py(2 hunks)src/mysql_to_sqlite3/transporter.py(3 hunks)tests/unit/test_mysql_utils.py(2 hunks)tests/unit/test_transporter.py(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: Test (python3.11, mysql:8.0, 0, false, 3.11)
- GitHub Check: Test (python3.12, mysql:5.5, 1, false, 3.12)
- GitHub Check: Test (python3.13, mariadb:10.4, 0, false, 3.13)
- GitHub Check: Test (python3.12, mariadb:10.4, 0, false, 3.12)
- GitHub Check: Test (python3.10, mariadb:10.5, 0, false, 3.10)
- GitHub Check: Test (python3.9, mariadb:10.6, 0, false, 3.9)
- GitHub Check: Test (python3.9, mariadb:10.5, 0, false, 3.9)
- GitHub Check: Test (python3.13, mariadb:10.5, 0, false, 3.13)
- GitHub Check: Test (python3.13, mariadb:10.3, 0, false, 3.13)
- GitHub Check: Test (python3.9, mariadb:10.4, 0, false, 3.9)
- GitHub Check: Test (python3.13, mariadb:10.1, 1, false, 3.13)
- GitHub Check: Test (python3.11, mariadb:10.2, 0, false, 3.11)
- GitHub Check: Test (python3.9, mariadb:10.3, 0, false, 3.9)
- GitHub Check: Test (python3.9, mariadb:10.2, 0, false, 3.9)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, false, 3.13)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, false, 3.11)
- GitHub Check: Test (python3.13, mariadb:10.0, 1, false, 3.13)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, false, 3.12)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, false, 3.9)
- GitHub Check: Test (python3.9, mariadb:5.5, 1, false, 3.9)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (7)
src/mysql_to_sqlite3/transporter.py (1)
21-21: LGTM!The import is correctly placed alongside the existing import from the same module.
src/mysql_to_sqlite3/mysql_utils.py (3)
46-116: Excellent defensive programming!The comprehensive row format handling ensures compatibility with different MySQL connector configurations and versions. This robust approach prevents potential runtime errors when dealing with various cursor types.
119-153: Well-implemented topological sort!The implementation correctly uses Kahn's algorithm and elegantly identifies cyclic edges by examining remaining dependencies. The dual return of sorted tables and cyclic edges provides valuable debugging information.
156-166: Clean abstraction with proper resource management!Good use of context manager for cursor handling. The function provides a clean interface for the transporter while ensuring proper resource cleanup.
tests/unit/test_mysql_utils.py (1)
177-421: Comprehensive test coverage!Excellent test suite covering:
- Acyclic and cyclic graphs
- Empty inputs
- Various row formats (tuples, lists, objects, dictionaries)
- Edge cases with None values
- Integration testing of the combined functionality
The tests effectively validate the robustness of the implementation.
tests/unit/test_transporter.py (2)
154-157: Good test enhancement for foreign key verification.The additions properly verify that foreign keys are re-enabled and checked after an exception, which aligns with the PR's focus on FK constraint handling.
233-427: Excellent test coverage for the new FK handling functionality.The three new tests comprehensively cover:
- Normal table ordering with FK constraints
- Handling of circular dependencies with appropriate warnings
- Graceful fallback when table ordering computation fails
The tests properly verify all the expected behaviors including foreign key pragma usage and logging.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Simplify type annotations for mock objects in unit tests by using implicit typing. Adjust list type in `transporter.py` to use `sqlite3.Row`.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/test_transporter.py (1)
364-427: Consider refactoring to reduce code duplication.Whilst the test correctly verifies the fallback behaviour when
compute_creation_orderfails, there's significant code duplication across the three new test methods. The repetitive setup could be extracted into a helper method or fixture.Consider creating a helper method for the common setup:
+ def _setup_transfer_test_mocks(self, mock_sqlite_connect, mock_mysql_connect): + """Helper method to set up common mocks for transfer tests.""" + # Setup mock SQLite cursor and connection + mock_sqlite_cursor = MagicMock() + mock_sqlite_connection = MagicMock() + mock_sqlite_connection.cursor.return_value = mock_sqlite_cursor + mock_sqlite_connect.return_value = mock_sqlite_connection + + # Setup mock MySQL cursor and connection + mock_mysql_cursor = MagicMock() + mock_mysql_cursor.fetchall.return_value = [(b"table1",), (b"table2",), (b"table3",)] + mock_mysql_connection = MagicMock() + mock_mysql_connection.cursor.return_value = mock_mysql_cursor + mock_mysql_connect.return_value = mock_mysql_connection + + # Create minimal instance + instance = MySQLtoSQLite() + instance._mysql = mock_mysql_connection + instance._mysql_tables = [] + instance._exclude_mysql_tables = [] + instance._mysql_cur = mock_mysql_cursor + instance._mysql_cur_dict = MagicMock() + instance._mysql_cur_prepared = MagicMock() + instance._sqlite_cur = mock_sqlite_cursor + instance._without_data = True + instance._without_tables = False + instance._without_foreign_keys = False + instance._vacuum = False + instance._logger = MagicMock() + instance._create_table = MagicMock() + + return instance, mock_sqlite_cursor, mock_mysql_connection
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/mysql_to_sqlite3/transporter.py(3 hunks)tests/unit/test_transporter.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mysql_to_sqlite3/transporter.py
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: Test (python3.13, mariadb:10.4, 0, false, 3.13)
- GitHub Check: Test (python3.12, mariadb:10.4, 0, false, 3.12)
- GitHub Check: Test (python3.10, mariadb:10.4, 0, false, 3.10)
- GitHub Check: Test (python3.13, mariadb:10.3, 0, false, 3.13)
- GitHub Check: Test (python3.11, mariadb:10.3, 0, false, 3.11)
- GitHub Check: Test (python3.12, mariadb:10.1, 1, false, 3.12)
- GitHub Check: Test (python3.9, mariadb:10.2, 0, false, 3.9)
- GitHub Check: Test (python3.12, mariadb:10.2, 0, false, 3.12)
- GitHub Check: Test (python3.11, mariadb:10.2, 0, false, 3.11)
- GitHub Check: Test (python3.13, mariadb:10.1, 1, false, 3.13)
- GitHub Check: Test (python3.10, mariadb:10.1, 1, false, 3.10)
- GitHub Check: Test (python3.12, mariadb:10.0, 1, false, 3.12)
- GitHub Check: Test (python3.11, mariadb:10.1, 1, false, 3.11)
- GitHub Check: Test (python3.9, mariadb:10.1, 1, false, 3.9)
- GitHub Check: Test (python3.12, mariadb:5.5, 1, false, 3.12)
- GitHub Check: Test (python3.9, mariadb:10.0, 1, false, 3.9)
- GitHub Check: Test (python3.13, mariadb:5.5, 1, false, 3.13)
- GitHub Check: Test (python3.11, mariadb:10.0, 1, false, 3.11)
- GitHub Check: Test (python3.11, mariadb:5.5, 1, false, 3.11)
- GitHub Check: Test (python3.10, mariadb:5.5, 1, false, 3.10)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
tests/unit/test_transporter.py (4)
2-2: LGTM! Type annotation import added correctly.The typing import is properly added to support type annotations used in the new test methods.
154-157: LGTM! Foreign key check verification added.The enhanced test correctly verifies that both foreign key re-enabling and foreign key constraint checking are performed after an exception occurs during transfer.
233-295: Well-structured test for table ordering functionality.The test comprehensively verifies the correct integration of
compute_creation_orderinto the transfer process, including proper mocking, order verification, and PRAGMA command execution.
296-362: Excellent test coverage for circular dependency handling.The test properly validates that circular dependencies are detected, logged as warnings, and that the transfer process continues with the computed order whilst maintaining proper foreign key constraint handling.
This pull request introduces enhancements to the MySQL-to-SQLite data transfer process, focusing on foreign key handling, schema metadata extraction, and table creation order computation. Additionally, unit tests have been expanded to ensure robust functionality and edge case coverage.
Enhancements to MySQL-to-SQLite data transfer:
Foreign key handling during transfer:
compute_creation_orderfunction. Tables are processed in a dependency-safe order, and cyclic dependencies are logged as warnings.PRAGMA foreign_key_check. Violations are logged for debugging purposes.Schema metadata extraction:
fetch_schema_metadatafunction to retrieve table names and foreign key relationships from the MySQL schema. This ensures robust handling of various row formats and edge cases.Codebase improvements:
topo_sort_tablesfunction to perform topological sorting of tables based on foreign key dependencies. This ensures tables are created in a foreign key-safe order. Cyclic dependencies are identified and returned for further analysis.Unit test enhancements:
topo_sort_tablesto verify behavior with acyclic, cyclic, and empty dependency graphs.fetch_schema_metadatato ensure robust handling of diverse row formats, including tuples, lists, objects, dictionaries, and invalid types.compute_creation_orderto validate table ordering and cycle detection in schemas with and without circular dependencies.transfermethod tests to verify foreign key checks and exception handling during SQLite operations.