Skip to content

Python(fix): tolerate multi-value metadata keys in metadata_proto_to_dict - #698

Draft
dengjonathan wants to merge 1 commit into
mainfrom
python-metadata-proto-to-dict-multivalue
Draft

Python(fix): tolerate multi-value metadata keys in metadata_proto_to_dict#698
dengjonathan wants to merge 1 commit into
mainfrom
python-metadata-proto-to-dict-multivalue

Conversation

@dengjonathan

Copy link
Copy Markdown
Contributor

What

metadata_proto_to_dict raises ValueError("Key already exists") when the repeated MetadataValue list contains the same key twice. Once the backend's org-scoped multi-value-metadata feature ships (azimuth ENG-9885), a metadata key can legitimately hold multiple values — and because Run._from_proto / Asset._from_proto call this helper, client.runs.get() / list_() crash outright for any entity carrying multi-value metadata. This also breaks azimuth's isolated rule runner and canvas parameter hydration, which fetch entities through this client.

This PR makes the helper keep the first value per key instead of raising. The API returns values in canonical order (the backend assigns order_index deterministically on write), so "first" is stable and matches the value every backend single-value context uses (e.g. azimuth's MetadataToFlatMap).

Why first-value rather than lists

Returning lists would change the type of .metadata values and break every existing caller. First-value keeps the dict view exactly as it is today for single-valued keys and degrades gracefully for multi-valued ones. Full list access (a dict-compatible Metadata mapping with getall(key), plus list writes) is scoped separately in Linear ENG-13281 — this PR is deliberately the minimal forward-compatibility fix.

Rollout note

This must be released before the multi-value-metadata flag is enabled for any org: every published sift-py version crashes on fetch the moment an org writes its first multi-value key. Tracked with an org-enablement checklist in ENG-13281.

Testing

New _tests/util/test_metadata.py (first test module for this util): value-type unwrapping, multi-value first-value behavior, empty input, and a dict→proto→dict round trip. ruff check and ruff format --check pass on the changed files.

Linear: ENG-13281 (client multi-value support), ENG-13104 (rule payload counterpart), ENG-9885 (feature)

🤖 Generated with Claude Code

…dict

A metadata key can hold multiple values once the multi-value-metadata
feature ships. metadata_proto_to_dict raised ValueError on the second
value of a key, which made Run/Asset._from_proto -- and therefore
client.runs.get()/list_() -- crash for any entity carrying multi-value
metadata. Keep the first value per key instead (the API returns values
in canonical order), matching the value backend single-value contexts
use.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Python docs preview: https://sift-stack.github.io/sift/python/pr-698/

Deployed from bfaea8f. The link may take up to a minute to become live as GitHub Pages propagates.

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.

1 participant