fix: 10 correctness bugs from full codebase audit
- immich_api: .get("items") or [] handles {"items": null} without crashing len()
- immich_api: catch TypeError alongside ValueError in filter_recent_assets for
timezone-naive fileCreatedAt comparisons
- scheduler: catch SystemExit in addition to KeyboardInterrupt so cli.main()
cannot kill the long-running scheduler process
- scheduler: reseed croniter from wall-clock time after each run so overrunning
jobs don't schedule an immediate back-to-back rerun
- config: reject negative YEARS_FILTER values with a warning, reset to default 10
- frigate_api: return True (not False) for empty filenames list — callers cannot
distinguish no-op from network failure on False
- jobs: warn on unrecognised STRATEGY value instead of silently falling back
- jobs: casefold ONLY_PEOPLE / SKIP_PEOPLE matching so "john doe" matches "John Doe"
- executor: <= → < so a same-score candidate can fill a freed replacement slot
- embeddings: set _insightface_loaded=True on GPU+CPU double-failure to prevent
N re-init attempts (one per asset) when InsightFace is broken for a whole run
This commit is contained in:
+2
-1
@@ -49,11 +49,12 @@ def _run_scheduler() -> None:
|
||||
try:
|
||||
main()
|
||||
print("winnow run complete", flush=True)
|
||||
except KeyboardInterrupt:
|
||||
except (KeyboardInterrupt, SystemExit):
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error("winnow run failed: %s", e, exc_info=True)
|
||||
print(f"winnow run failed: {e}", flush=True)
|
||||
cron = croniter(schedule, time.time())
|
||||
next_run = cron.get_next(float)
|
||||
print(f"Next run: {time.strftime('%Y-%m-%d %H:%M:%S', time.localtime(next_run))}", flush=True)
|
||||
time.sleep(min(60, max(1, next_run - time.time())))
|
||||
|
||||
@@ -127,6 +127,9 @@ class _Config:
|
||||
self.API_KEY = os.getenv("API_KEY")
|
||||
self.OUTPUT_DIR = os.getenv("OUTPUT_DIR", "./frigate_train")
|
||||
self.YEARS_FILTER = _getenv_int("YEARS_FILTER", 10)
|
||||
if self.YEARS_FILTER < 0:
|
||||
logging.warning("YEARS_FILTER=%s is negative — using default 10", self.YEARS_FILTER)
|
||||
self.YEARS_FILTER = 10
|
||||
self.MIN_FACE_WIDTH = _getenv_int("MIN_FACE_WIDTH", 90)
|
||||
self.MIN_FACE_COUNT = _getenv_int("MIN_FACE_COUNT", 3)
|
||||
self.MERGE_DUPLICATE_PEOPLE = _getenv_bool("MERGE_DUPLICATE_PEOPLE", False)
|
||||
|
||||
@@ -107,7 +107,6 @@ def get_insightface_app():
|
||||
global _insightface_app, _insightface_loaded
|
||||
if _insightface_loaded:
|
||||
return _insightface_app
|
||||
_insightface_loaded = True
|
||||
|
||||
ctx_id = -1
|
||||
insightface_home = os.environ.get("INSIGHTFACE_HOME", os.path.expanduser("~/.insightface"))
|
||||
@@ -172,10 +171,12 @@ def get_insightface_app():
|
||||
_insightface_app.prepare(ctx_id=ctx_id, det_size=(640, 640))
|
||||
|
||||
logger.info("InsightFace Buffalo_L: ready on %s (%.1fs)", device_str, time.time() - t0)
|
||||
_insightface_loaded = True
|
||||
return _insightface_app
|
||||
|
||||
except ImportError:
|
||||
logger.error("InsightFace not installed!")
|
||||
_insightface_loaded = True
|
||||
return None
|
||||
except Exception as e:
|
||||
logger.error("Failed to load InsightFace: %s", e)
|
||||
@@ -193,9 +194,11 @@ def get_insightface_app():
|
||||
)
|
||||
_insightface_app.prepare(ctx_id=-1, det_size=(640, 640))
|
||||
logger.info("InsightFace Buffalo_L: ready on CPU (fallback, %.1fs)", time.time() - t0)
|
||||
_insightface_loaded = True
|
||||
return _insightface_app
|
||||
except Exception as ex:
|
||||
logger.error("InsightFace CPU fallback failed: %s", ex)
|
||||
_insightface_loaded = True
|
||||
return None
|
||||
|
||||
|
||||
|
||||
+2
-2
@@ -384,9 +384,9 @@ def upload_to_frigate(jobs: list[dict]) -> None:
|
||||
# freed slot isn't filled with something worse than what we removed.
|
||||
if min_quality_score_for_slot is not None:
|
||||
file_score = score_map.get(fname)
|
||||
if file_score is not None and file_score <= min_quality_score_for_slot:
|
||||
if file_score is not None and file_score < min_quality_score_for_slot:
|
||||
progress.console.print(
|
||||
f" [dim]⏭ {fname}: score {file_score:.3f} ≤ freed slot floor"
|
||||
f" [dim]⏭ {fname}: score {file_score:.3f} < freed slot floor"
|
||||
f" {min_quality_score_for_slot:.3f}, skipping[/dim]"
|
||||
)
|
||||
progress.advance(upload_task)
|
||||
|
||||
@@ -146,8 +146,10 @@ def delete_frigate_person_files(person_name: str, filenames: list[str]) -> bool:
|
||||
Returns True on success, False if unreachable or the request fails.
|
||||
"""
|
||||
frigate_url = _get_frigate_url()
|
||||
if not frigate_url or not filenames:
|
||||
if not frigate_url:
|
||||
return False
|
||||
if not filenames:
|
||||
return True
|
||||
from urllib.parse import quote
|
||||
encoded_name = quote(person_name, safe="")
|
||||
try:
|
||||
|
||||
@@ -134,7 +134,7 @@ def fetch_all_assets(person: dict) -> tuple[list[dict], int]:
|
||||
# Immich ≥2.x returns {"assets": {"items": [...]}};
|
||||
# earlier versions returned {"assets": [...]} directly.
|
||||
if isinstance(page_assets, dict):
|
||||
page_assets = page_assets.get("items", [])
|
||||
page_assets = page_assets.get("items") or []
|
||||
|
||||
page_count = len(page_assets) # raw count for termination check before filtering
|
||||
|
||||
@@ -303,7 +303,7 @@ def filter_recent_assets(assets: list[dict], years: int | None = None) -> list[d
|
||||
recent.append(asset)
|
||||
else:
|
||||
skipped += 1
|
||||
except ValueError:
|
||||
except (ValueError, TypeError):
|
||||
bad_timestamp += 1
|
||||
continue
|
||||
|
||||
|
||||
+9
-5
@@ -86,7 +86,11 @@ def _resolve_strategy(strategy: str, has_embedding: bool) -> tuple[int | str, st
|
||||
"standard": (30, "smart"),
|
||||
"broad": (100, "smart"),
|
||||
}
|
||||
return strategy_map.get(strategy, ("auto", "smart"))
|
||||
result = strategy_map.get(strategy)
|
||||
if result is None:
|
||||
logger.warning("Unrecognised STRATEGY=%r — falling back to auto", strategy)
|
||||
return ("auto", "smart")
|
||||
return result
|
||||
|
||||
|
||||
def _perform_selection(
|
||||
@@ -244,13 +248,13 @@ def auto_configure(people: list[dict]) -> list[dict]:
|
||||
return []
|
||||
|
||||
strategy = os.environ.get("STRATEGY", "auto")
|
||||
skip = [s.strip() for s in os.environ.get("SKIP_PEOPLE", "").split(",") if s.strip()]
|
||||
only = [s.strip() for s in os.environ.get("ONLY_PEOPLE", "").split(",") if s.strip()]
|
||||
skip = {s.strip().casefold() for s in os.environ.get("SKIP_PEOPLE", "").split(",") if s.strip()}
|
||||
only = {s.strip().casefold() for s in os.environ.get("ONLY_PEOPLE", "").split(",") if s.strip()}
|
||||
|
||||
if only:
|
||||
valid_people = [p for p in valid_people if p["name"] in only]
|
||||
valid_people = [p for p in valid_people if p["name"].casefold() in only]
|
||||
if skip:
|
||||
valid_people = [p for p in valid_people if p["name"] not in skip]
|
||||
valid_people = [p for p in valid_people if p["name"].casefold() not in skip]
|
||||
|
||||
min_face_count = Config.MIN_FACE_COUNT
|
||||
|
||||
|
||||
Reference in New Issue
Block a user