feat(analytics): log an error when an Event class was instantiated that lacks the eventclass decorator #98922
Conversation
…at lacks the eventclass decorator
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
src/sentry/analytics/event.py
Outdated
| current_annotations = getattr(cls, "__annotations__", {}) | ||
|
|
||
| # Filter out ClassVar annotations as they're not instance fields | ||
| current_fields: set[str] = { |
There was a problem hiding this comment.
what happens if the fields are not annotated?
Is there a way to force annotations?
There was a problem hiding this comment.
what happens if the fields are not annotated?
Then they don't show up here. Not sure how and why that would happen, though.
Is there a way to force annotations?
No, I don't think so
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| class ExampleEventWithoutEventclass(ExampleEvent): | ||
| new_field: str = "test" | ||
|
|
||
| with pytest.raises(TypeError): |
There was a problem hiding this comment.
why are we testing for this?
The code doesn't seem to introduce the throwing of an error, why is this triggered in tests?
There was a problem hiding this comment.
It actually does, because we are using a subclass of a dataclass without adding the dataclass or in our cases eventclass decorator. This is why there is an outdated constructor with the new_field element missing.
constantinius
deleted the
constantinius/feat/analytics/log-error-when-no-decorator
branch
Closes https://linear.app/getsentry/issue/TET-808/analytics-investigate-whether-and-how-to-force-use-of-eventclass