fix: correct reconcile log severity, docstring gaps, and _entry allocation (v0.5.9)
- reconcile.py: swap log levels — external-upload path (permanent mapping loss) escalated to WARNING; timeout path (transient, retries next cycle) downgraded to INFO. Also extend the warning message to note the files are permanently unmapped. - immich_api.py: extend fetch_all_assets docstring to document that all-garbage page termination (in addition to network errors) makes total_raw a lower bound. - executor.py: add comment above shutil.rmtree noting that POSIX rmtree raises NotADirectoryError on a top-level symlink, documenting why the removed islink guard is safe to omit. - upload_tracker.py: replace setdefault with explicit guard in _entry() — setdefault evaluates its default-dict argument before checking key presence, allocating and discarding a dict on every already-present call.
This commit is contained in:
@@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [0.5.9] - 2026-06-15
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Reconcile log severity corrected**: the external-upload branch (`len(new_files) > target`) was logged at `INFO` while the timeout branch (`< target`) was logged at `WARNING`. The severity is now inverted to match impact: external upload causes permanent mapping loss (those files are never eligible for quality replacement) and is now `WARNING`; timeout is transient and recoverable next cycle and is now `INFO`.
|
||||
|
||||
- **`fetch_all_assets` docstring: lower-bound caveat now covers both interruption cases**: previously only noted that a network error makes `total_raw` a lower bound. An all-garbage page (every item non-dict) also terminates pagination early, leaving later pages unfetched — this case is now documented alongside the network error case.
|
||||
|
||||
- **`shutil.rmtree` symlink safety documented**: added a comment above the `rmtree` call in `execute_jobs` noting that POSIX `shutil.rmtree` raises `NotADirectoryError` on a top-level symlink, so a race-replaced symlink cannot cause out-of-tree deletion.
|
||||
|
||||
- **`_entry()` in `get_person_summary` no longer allocates default dict for present keys**: `setdefault` evaluates its default argument before checking whether the key exists, allocating and immediately discarding a 5-key dict on every call for an already-present person. Replaced with an explicit `if name not in summary` guard.
|
||||
|
||||
## [0.5.8] - 2026-06-15
|
||||
|
||||
### Fixed
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "winnow"
|
||||
version = "0.5.8"
|
||||
version = "0.5.9"
|
||||
description = "Selects diverse, high-quality photos from Immich as training data for Frigate face recognition."
|
||||
license = "AGPL-3.0-or-later"
|
||||
requires-python = ">=3.13"
|
||||
|
||||
@@ -107,6 +107,8 @@ def execute_jobs(jobs: list[dict]) -> None:
|
||||
logger.error(str(e))
|
||||
continue
|
||||
# Face crops are transient (uploaded then discarded); wipe before each run.
|
||||
# shutil.rmtree raises NotADirectoryError on a top-level symlink (POSIX),
|
||||
# so a race-replaced symlink cannot cause deletion outside output_dir.
|
||||
if os.path.isdir(person_dir):
|
||||
shutil.rmtree(person_dir)
|
||||
os.makedirs(person_dir, exist_ok=True)
|
||||
|
||||
@@ -89,8 +89,9 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
|
||||
Returns (assets, total_raw) where assets is the list of valid dict items
|
||||
and total_raw is the raw item count across pages that had at least one valid
|
||||
dict. All-garbage pages (every item non-dict) stop pagination and are not
|
||||
counted. If pagination is interrupted by a network error, total_raw is a
|
||||
lower bound (a warning is logged).
|
||||
counted. total_raw is a lower bound in two cases: a network error interrupts
|
||||
pagination (a warning is logged), or an all-garbage page terminates it early
|
||||
(a warning is logged and later pages are not fetched).
|
||||
"""
|
||||
name = person.get("name", "Unknown")
|
||||
person_id = person.get("id")
|
||||
|
||||
+4
-3
@@ -76,15 +76,16 @@ def reconcile_frigate_mappings(
|
||||
}
|
||||
record_frigate_files_batch(person_name, mappings)
|
||||
elif len(new_files) > target:
|
||||
logger.info(
|
||||
logger.warning(
|
||||
"%s: %s new Frigate files for %s uploads"
|
||||
" (external upload detected) — skipping file mapping",
|
||||
" (external upload detected) — skipping file mapping;"
|
||||
" these files are permanently unmapped",
|
||||
person_name,
|
||||
len(new_files),
|
||||
target,
|
||||
)
|
||||
else:
|
||||
logger.warning(
|
||||
logger.info(
|
||||
"%s: only %s of %s expected Frigate files"
|
||||
" appeared after reconciliation — mapping skipped",
|
||||
person_name,
|
||||
|
||||
@@ -443,9 +443,9 @@ def get_person_summary() -> dict[str, dict]:
|
||||
).fetchall()
|
||||
|
||||
def _entry(summary: dict, name: str) -> dict:
|
||||
return summary.setdefault(
|
||||
name, {"uploaded": 0, "rejected": 0, "frigate_count": None, "scores": {}, "frigate_files": {}}
|
||||
)
|
||||
if name not in summary:
|
||||
summary[name] = {"uploaded": 0, "rejected": 0, "frigate_count": None, "scores": {}, "frigate_files": {}}
|
||||
return summary[name]
|
||||
|
||||
summary: dict[str, dict] = {}
|
||||
for r in rows:
|
||||
|
||||
Reference in New Issue
Block a user