refactor: simplify sync logic and remove unused pydantic dependency#8
Merged
Conversation
- Add hash verification (get_file_hashes, verify_sync_integrity) - Add commit message marker parsing/appending (parse_marker, append_marker_to_commit, find_last_synced_sha) - Add locking mechanism (check_sync_lock, acquire_sync_lock, release_sync_lock) - Add corresponding unit tests
Split into: - verify.py: hash verification (get_file_hashes, verify_sync_integrity) - marker.py: commit message markers (parse_marker, append_marker_to_commit, find_last_synced_sha) - lock.py: locking mechanism (check_sync_lock, acquire_sync_lock, release_sync_lock) - sync.py: main sync logic, re-exports for backwards compatibility
- Remove __all__ re-export list from sync.py - Update test imports to use new module locations
Add new CLI options: - --marker-prefix: prefix for sync marker in commit messages (default: synced) - --reset: reset sync state and re-sync all commits from beginning Update sync() function signature to accept new parameters.
Add tests for: - Marker in commit message - Idempotent sync (no duplicates) - New commits only on subsequent syncs - Reset flag functionality - Hash verification - Lock checking Some tests expected to fail until idempotency is wired in.
- Add locking check before sync - Add marker rewriting after filter - Add helper functions for finding last synced SHA - Basic idempotency working (markers, no duplicates) - Remaining: incremental sync (filter only new commits)
Production code: - Remove _clone_commits_since (dead helper, logic already inline) - Remove acquire_sync_lock / release_sync_lock (never called in production) - Remove double fetch: check_sync_lock no longer fetches (caller does it) - Remove extraneous fetch in push_to_remote dry-run path - Use append_marker_to_commit from marker.py instead of inline duplicate - Add _decode_message helper to deduplicate bytes/str decode pattern - Remove last_synced_sha from SyncResult (was computed but never read) - Replace SyncConfig Pydantic model with a simple _validate_branch helper (the model was a 1:1 pass-through adding a pydantic dependency with no real behaviour; the glob_translate validator accepted everything) Tests: - Add tests/integration/conftest.py with git_env fixture and run_git helper - Update integration tests to use conftest instead of copy-pasted env blocks - Fix test_verify_sync_integrity to use separate mocks per repo (was passing same mock as both private and public, not testing two-repo comparison) - Fix test_sync_lock: remove tests for dead acquire/release stubs, add a meaningful third case (dest branch != lock branch) - Fix reset test: assert marker present rather than SHA changed (SHAs are deterministic from identical content so the previous assertion was wrong)
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
1 participant
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.
Summary
pydanticdependency (was never imported in the codebase)sync.py: removes the redundant remote setup block that duplicated whatpush_to_remotealready does, simplifies theprivate_repo.close()call by dropping the unnecessaryhasattrguard, and collapses the doublereturnat the end into a single returnTrue if last_synced_sha else force→force or bool(last_synced_sha)_decode_messagehelper and inlines the bytes/str decode at its two call sitesfind_last_synced_shainmarker.pyto usenext()with a walrus operator instead of a manualforloopAll 14 integration tests pass unchanged.