Skip to content

fix(schema): enforce and document metadata.tags cardinality limit - #25

Open
dacostafrankaboagye wants to merge 1 commit into
flameiq:mainfrom
dacostafrankaboagye:fix/17-tags-cardinality-validation-rule
Open

fix(schema): enforce and document metadata.tags cardinality limit#25
dacostafrankaboagye wants to merge 1 commit into
flameiq:mainfrom
dacostafrankaboagye:fix/17-tags-cardinality-validation-rule

Conversation

@dacostafrankaboagye

Copy link
Copy Markdown

Summary

Closes #17.

The schema spec's field reference states metadata.tags has a "Maximum
50 entries" constraint, but the formal Validation rules section (items
1–6) never listed it, and — more importantly — the constraint wasn't
actually enforced in code at all
. PerformanceSnapshot.from_dict()
would silently accept a tags dict of any size or key/value type.

Changes

  • flameiq/schema/v1/models.py — added SnapshotMetadata.__post_init__,
    following the same pattern as LatencyMetrics/Metrics elsewhere in
    this module. Raises ValueError if tags has more than 50 entries,
    or if any key/value is not a str.
  • docs/source/specs/schema-v1.rst — added validation rule 7,
    matching the field reference, and clarified that violating it raises
    ValueError (not SchemaVersionError — that class is reserved for
    unsupported schema_version values and isn't used elsewhere in this
    module; every other check here raises plain ValueError).
  • tests/unit/test_schema/test_models.py — added TestSnapshotMetadata
    covering: exactly 50 tags (valid boundary), 51 tags (raises), non-string
    key (raises), non-string value (raises).

Acceptance criteria (from #17)

  • Add tags cardinality to the formal validation rules section
  • Add key/value type constraints to the same rule
  • Verify the implementation actually enforces this and add a unit
    test if missing — it did not enforce it at all; now it does
  • Clarify whether exceeding the limit raises SchemaVersionError
    or a different exception — clarified as ValueError, for
    consistency with the rest of the module

Testing

  • pytest tests/unit/test_schema/test_models.py -v — all 19 tests pass
    (4 new)
  • Manually verified via flameiq validate with a metrics file
    containing 55 tags (fails with the new error message) and 45 tags
    (passes)

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