[python] Align SnapshotManager constructor with Java#7758
Open
TheR1sing3un wants to merge 1 commit intoapache:masterfrom
Open
[python] Align SnapshotManager constructor with Java#7758TheR1sing3un wants to merge 1 commit intoapache:masterfrom
TheR1sing3un wants to merge 1 commit intoapache:masterfrom
Conversation
Switch SnapshotManager.__init__ from (table) to the Java-aligned (file_io, table_path, branch=None, snapshot_loader=None) so the class no longer depends on FileStoreTable. The new manager carries its own file_io / table_path / branch / snapshot_loader fields, mirroring paimon-core/.../utils/SnapshotManager.java. FileStoreTable.snapshot_manager() remains the canonical factory and now wires those four basics. All raw SnapshotManager(table) call sites across production and tests are migrated to table.snapshot_manager(). copy_with_branch is rewritten to construct the rebranched manager directly via the new constructor (no field-swap). Mock-style tests that patched the SnapshotManager class to intercept its instances now wire the mock through table.snapshot_manager.return_value, which matches how production code obtains its instance. Follow-up to apache#7756.
d0aa481 to
7e1153a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Switch the Python
SnapshotManager.__init__from(table)to the Java-aligned(file_io, table_path, branch=None, snapshot_loader=None), so the class no longer depends onFileStoreTable. Mirrorspaimon-core/.../utils/SnapshotManager.java's constructor (which takesFileIO, Path tablePath, @Nullable String branchName, @Nullable SnapshotLoader, @Nullable Cache); theCacheparameter is left out of scope (Caffeine has no Python equivalent yet).The previous Python signature held
self.table = tableand reached throughself.table.file_io/self.table.catalog_environment.snapshot_loader()/self.table.current_branch()to populate fields. That madeSnapshotManagera layering-violation -- the snapshot layer reaching back into the table layer -- and forced mock tests to stub a four-attribute mock table just to construct the manager. The new constructor takes the four basics directly.In scope
pypaimon/snapshot/snapshot_manager.py: new constructor;self.tablefield removed;copy_with_branchrewritten to construct the rebranched manager via the new constructor (no field-swap).pypaimon/table/file_store_table.py:snapshot_manager()remains the canonical factory and now wires(file_io, table_path, current_branch(), catalog_environment.snapshot_loader())directly.table.snapshot_manager().streaming_table_scan_test.py/file_store_commit_test.py/partition_predicate_test.py: drop the@patch('...SnapshotManager')decorator and the mock parameter; wire each test'smock_snapshot_managerthroughtable.snapshot_manager.return_value, matching how production code obtains its instance.snapshot_manager_test.py: simplified mock setUp -- no more layered `Mock()` table, just `SnapshotManager(file_io, "/tmp/test_table")`.Out of scope
Tests
From `paimon-python/`:
```
pytest pypaimon/tests/snapshot_manager_test.py pypaimon/tests/branch_manager_test.py \
pypaimon/tests/streaming_table_scan_test.py pypaimon/tests/file_store_commit_test.py \
pypaimon/tests/partition_predicate_test.py -q # 76 passed
pytest pypaimon/tests/ --ignore=pypaimon/tests/rest --ignore=pypaimon/tests/e2e_rest \
--ignore=pypaimon/tests/sql_context_test.py # 963 passed
flake8 --config=dev/cfg.ini # clean
```
(`sql_context_test.py` requires the optional `datafusion` extra which isn't installed in this environment; failures there are pre-existing on master and unrelated to this diff.)
Anti-divergence checklist
Generative AI disclosure
Drafted with assistance from a generative AI tool. All code, tests, and Java alignment were reviewed and validated by the contributor.
Note: built on top of #7756. Once #7756 merges, this branch will rebase cleanly onto master.