Refactor: Architecture cleanup, performance fixes, and robust logging - #43
Merged
Conversation
Moves the low-level `_is_service_enabled` logic from the presentation layer (`cli.py`) to the service management module (`service.py`). This fixes a Separation of Concerns violation where the CLI was directly managing OS-specific system calls (systemd/launchd) instead of delegating to the appropriate backend module.
Extracts the repeated logic for reading and stripping lines from the registry file into a reusable `get_registered_repos` helper in `system.py`. Applied this helper across `cli.py` and `daemon.py`.
Replaces manual git plumbing in `daemon.py` with calls to `GitRepo` methods (`commit_tree`, `update_ref`) and `ops.get_backup_ref`. This ensures consistency in backup reference generation and reduces code duplication.
Adds logging to `GitRepo` methods (`rev_parse`, `list_refs`, `get_last_commit_time`) that previously swallowed exceptions silently. This improves visibility into git corruption or runtime errors.
Ensures that battery checks, notifications, and machine ID resolution failures are logged rather than silently ignored. This prevents cases where the daemon might drain the battery or fail to identify the machine without any trail.
Adds explicit logging to `_get_ref_timestamp` and `has_large_files` to catch and report specific errors instead of swallowing them.
Modifies the `sync_session` operation to fetch backup references only for the current branch, rather than fetching the entire backup namespace. This reduces network usage and fetch time.
Removes the 1-second `time.sleep` call when a git lock file is detected. The daemon will now fail fast for that specific repository and proceed to the next one, rather than blocking the entire loop.
Implements a caching mechanism in `Config.load` to read the global configuration file only once per process, significantly reducing disk I/O when iterating over multiple repositories in the daemon.
Updates `show_status` to use `system.get_registered_repos()` instead of manually parsing the registry file. This ensures consistent logic across the application and respects the new architectural boundary.
Updates the test suite to align with recent architectural and performance changes: - test(ops): update sync test to expect optimized single-branch fetch. - test(cli): mock `system.get_registered_repos` dependencies correctly. - test(config): add fixture to clear global config cache between tests. - test(git): add coverage for silent failure logging in `GitRepo`. - fix(test): patch registry constant in `setup_repo` test to prevent leaking test paths into the user's real registry file.
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
This PR addresses technical debt by enforcing strict separation of concerns between the CLI and Service layers, eliminating silent failures via robust logging, and optimizing resource-intensive operations.
Key Changes
🏗 Architecture & DRY
systemd/launchd) fromcli.pytoservice.py. The CLI now queries the service module rather than executing system calls directly.system.get_registered_repos(), removing duplicated file-reading code acrosscli.pyanddaemon.py.🚀 Performance
git pulsar syncnow fetches only the specific branch history (refs/heads/wip/pulsar/*/<branch>) instead of the entire backup namespace, significantly reducing network usage.Config.load()now caches the global configuration in memory, reducing disk I/O during the daemon's repetitive loop.time.sleep(1.0)in the daemon's lock detection, allowing it to fail fast and proceed to other repositories instead of stalling the entire process.🛡 Robustness & Logging
GitRepomethods (rev_parse,list_refs) andSystemStrategy(battery/notifications) that previously swallowed exceptions silently.🧪 Tests
test_cli.pyregarding registry mocking.clear_config_cachefixture to prevent test pollution.REGISTRY_FILEintest_setup_repo_triggers_identity_configto prevent tests from writing junk paths to the user's actual~/.local/state/git-pulsar/registry.Checklist
uv run pytest)