Skip to content

Fix metrics view annotation validation checking the metrics view's model/table instead of the annotation's#9724

Open
nishantmonu51 wants to merge 1 commit into
mainfrom
nishant/annotation-validation
Open

Fix metrics view annotation validation checking the metrics view's model/table instead of the annotation's#9724
nishantmonu51 wants to merge 1 commit into
mainfrom
nishant/annotation-validation

Conversation

@nishantmonu51

Copy link
Copy Markdown
Collaborator
  • The "must set exactly one of model/table" checks inside the annotations loop in parse_metrics_view.go read the metrics view's top-level model/table fields instead of the annotation's own fields.
  • As a result, an annotation that set neither field passed validation and emitted a spec annotation with an empty name, model, and table; and annotations on parent-based derived metrics views were always rejected, since derived views cannot set a data source.
  • Changed the checks to validate annotation.model/annotation.table, and added parser tests for both error cases plus annotations on a derived metrics view.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

…ble instead of the annotation's

The "exactly one of model/table" checks inside the annotations loop read the
metrics view's top-level fields, so an annotation that set neither field passed
validation silently, and annotations on parent-based derived metrics views were
always rejected. Check the annotation's own fields instead.
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