Skip to content

fix(model-store): specify UTF-8 encoding in registry read_text to prevent UnicodeDecodeError - #5881

Closed
vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/register-model-store-encoding
Closed

vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/register-model-store-encoding

Conversation

@vaibhavsrv

Copy link
Copy Markdown
Contributor

Why this matters

In ods/scripts/register-model-store.py, register() reads the existing model store registry via registry_path.read_text() without specifying an explicit encoding. On Python's default behavior, read_text() uses the locale-preferred encoding — cp1252 on Windows, ascii in some minimal containers — rather than UTF-8. When a registry file contains non-ASCII characters in stored host paths (e.g. paths with accented characters or emoji), read_text() raises an unhandled UnicodeDecodeError, aborting model store registration even though the registry was written correctly as UTF-8.

This patch adds encoding='utf-8' to the registry_path.read_text() call at line 50, matching the encoding used by all other JSON reads in the script and ensuring portable, platform-independent registry access.

Validation

  • Repro baseline: Wrote a registry file containing a non-ASCII hostPath (/models/café-models) as UTF-8 bytes; confirmed that read_text() without encoding raised UnicodeDecodeError in locales where the default encoding is not UTF-8.
  • Post-fix: Verified ods/tests/test_register_model_store_encoding.py passes with exit code 0; non-ASCII paths round-trip correctly and the missing-file fallback continues to produce the default empty registry.
  • Model store suites: 3 passed. New-test syntax and diff checks pass; new regressions wired into Linux CI.

Overlap check

Risk / AI disclosure

AI-assisted investigation, implementation and CLI regressions. This strengthens portable JSON reading in model store registration; runtime daemon behavior, docker execution, and installed model catalogs are not altered. Independent human review and platform/runtime qualification remain gates. No running configuration, deployment or upstream merge changed.

Follow-up integration evidence

Composed with #5876 at d1f6b0b06 without conflicts. Production and test diffs passed together; adjacent model store compose flags and registry contracts remain intact.
Backlog composition was local-only (production/test diffs, excluding workflow/Makefile wiring); it is not an upstream merge or independent human approval. Declared live-review gates remain open.

@Lightheartdevs

Copy link
Copy Markdown
Collaborator

Thanks for this contribution. public-beta was promoted into main on 2026-09-24 and no longer receives changes, so we're closing pull requests that target it. This isn't a judgment on the change itself. If it's still needed, please rebase onto main and open a focused PR. See #7253 for details and the contribution policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants