feat: Add --force flag to usecases get command for forced updates - #57
Open
dandye wants to merge 6 commits into
Open
feat: Add --force flag to usecases get command for forced updates#57dandye wants to merge 6 commits into
dandye wants to merge 6 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
dandye
force-pushed
the
kenn-forge/issue-38-feat-get-needs-a-force-command-for-usecase-updates
branch
from
August 18, 2026 03:09
e82a55d to
5a9e399
Compare
Adds optional --force (-f) flag to 'logstory usecases get' command to enable overwriting existing usecases with latest remote versions. Changes: - Modified _download_usecase() to accept optional force parameter - Added skip logic when force=False and usecase already installed - Added --force/--force option to usecase_get() CLI command - Updated docs/cli-reference.md with new flag documentation - Added comprehensive unit tests in tests/test_usecases_get_force.py - Updated CHANGELOG.md with feature entry Behavior: - Without --force: skips if usecase already installed (existing behavior) - With --force: overwrites all existing usecase files with latest remote - Backward compatible: all existing calls work unchanged Resolves Issue #38
Fixes two issues identified by roborev: 1. Medium severity: Force-update wasn't deleting removed files - When force=True, now removes the entire usecase directory before downloading - Ensures true sync behavior, not just a superset merge - Added shutil.rmtree() call before blob download loop 2. Low severity: Tests depended on environment configuration - Now mock get_usecases_buckets() in all force logic tests - Tests are now self-contained and environment-independent - Prevents test failures if LOGSTORY_USECASES_BUCKETS is unset All tests now properly verify force and non-force paths.
Addresses roborev review findings: 1. Moved blob list retrieval BEFORE deletion to prevent data loss - If _get_blobs() fails, no files are deleted - Only files not in the new blob list are removed after validation - Preserves safety: partial failures don't wipe the usecase - Proper cleanup of empty directories after file removal 2. Enhanced test to verify stale file deletion behavior - Test now pre-creates a stale file not in blob_list - Asserts the stale file is actually removed by force=True - Ensures the test catches regressions in cleanup logic This approach is safer and more fault-tolerant than delete-then-fetch.
Fixes critical bugs found by roborev: 1. High severity: Fixed path-prefix mismatch in stale file detection - Was comparing 'usecases/TEST_USECASE/file.log' against 'TEST_USECASE/file.log' - Now correctly compute relative path from usecases directory to match blob.name - Only files NOT in blob_list are deleted (true stale file removal) 2. Low severity: Fixed empty directory cleanup - Changed os.walk to topdown=False so children are processed before parents - Directories that become empty from file removal are now properly cleaned up - Check directory existence before attempting rmdir (defensive) 3. Enhanced test to verify selective file removal - Test now creates two files: one in blob_list (kept_file), one stale - Verifies stale file is deleted while kept file is preserved/re-downloaded - Tests the exact scenario that was broken: partial failure safety
Addresses remaining roborev findings: 1. Low severity: Added Windows path separator normalization - GCS blob.name always uses forward slashes - os.path.relpath uses os.sep (backslash on Windows) - Normalize with replace(os.sep, '/') before path comparison - Ensures correct path matching on all platforms 2. Low severity: Improved test to verify true file preservation - Removed download side_effect that masked incorrect deletion - Now the mocked download is a no-op - Test verifies file was never deleted (original content unchanged) - Catches regressions if path comparison logic breaks again Test now rigorously proves: stale files deleted, kept files preserved.
Removes leftover kept_file_stat variable that was computed but never used.
dandye
force-pushed
the
kenn-forge/issue-38-feat-get-needs-a-force-command-for-usecase-updates
branch
from
August 18, 2026 03:10
5a9e399 to
f647dff
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
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.
Description
Adds optional
--force/-fflag to thelogstory usecases getcommand to enable forced updates of existing usecases.Changes
_download_usecase()to accept optionalforce: bool = Falseparameter--force/-fCLI flag via typer.OptionBehavior
Testing
All changes verified by roborev code review (PASSED with no findings)
Related Issue
Resolves #38