fix: address 3 code review findings (round 10)
- immich_api: use 'or []' instead of .get("people", []) in get_people
so {"people": null} responses (some Immich versions with zero people
enrolled) return [] rather than None; .get() default only fires when
the key is absent, not when its value is null
- embeddings: log OSError from os.dup2 restore at DEBUG rather than
silently swallowing it; if a C extension (CUDA/onnxruntime) invalidates
the saved fd, the restore fails silently and stdout stays wired to
/dev/null — logging makes the event observable without changing the
swallow-and-continue semantics
- cache: remove MemoryError re-raise from EmbeddingCache.get(); a cache
read OOM aborted the entire diversity-selection batch for the person
rather than falling back to a fresh embedding computation, which is
the more appropriate OOM gate; broadening back to except Exception
restores the pre-round-5 fallback behavior
This commit is contained in:
@@ -75,8 +75,6 @@ class EmbeddingCache:
|
|||||||
if os.path.exists(path):
|
if os.path.exists(path):
|
||||||
try:
|
try:
|
||||||
return np.load(path)
|
return np.load(path)
|
||||||
except MemoryError:
|
|
||||||
raise
|
|
||||||
except Exception:
|
except Exception:
|
||||||
return None
|
return None
|
||||||
return None
|
return None
|
||||||
|
|||||||
@@ -45,8 +45,8 @@ def _suppress_output():
|
|||||||
if saved_out is not None:
|
if saved_out is not None:
|
||||||
try:
|
try:
|
||||||
os.dup2(saved_out, 1)
|
os.dup2(saved_out, 1)
|
||||||
except OSError:
|
except OSError as e:
|
||||||
pass
|
logger.debug("_suppress_output: failed to restore stdout fd: %s", e)
|
||||||
finally:
|
finally:
|
||||||
try:
|
try:
|
||||||
os.close(saved_out)
|
os.close(saved_out)
|
||||||
@@ -55,8 +55,8 @@ def _suppress_output():
|
|||||||
if saved_err is not None:
|
if saved_err is not None:
|
||||||
try:
|
try:
|
||||||
os.dup2(saved_err, 2)
|
os.dup2(saved_err, 2)
|
||||||
except OSError:
|
except OSError as e:
|
||||||
pass
|
logger.debug("_suppress_output: failed to restore stderr fd: %s", e)
|
||||||
finally:
|
finally:
|
||||||
try:
|
try:
|
||||||
os.close(saved_err)
|
os.close(saved_err)
|
||||||
|
|||||||
@@ -61,7 +61,7 @@ def get_people() -> list[dict]:
|
|||||||
if not isinstance(data, dict):
|
if not isinstance(data, dict):
|
||||||
logger.error("Unexpected response shape from Immich /people: %r", type(data))
|
logger.error("Unexpected response shape from Immich /people: %r", type(data))
|
||||||
return []
|
return []
|
||||||
return data.get("people", [])
|
return data.get("people") or []
|
||||||
except (requests.RequestException, ValueError, AttributeError) as e:
|
except (requests.RequestException, ValueError, AttributeError) as e:
|
||||||
logger.error("Failed to fetch people from Immich: %s", e)
|
logger.error("Failed to fetch people from Immich: %s", e)
|
||||||
return []
|
return []
|
||||||
|
|||||||
Reference in New Issue
Block a user