You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
VSCode/pyright does error on the above line because it might crash. Unfortunately, mypy does not.
We had an incident (without incident ticket) where this exact crash happened: c19ad27
This schema change added a new, optional field, but the corresponding Snuba PR accessed the field as if it was required. Mypy did not catch this.
When we merged the Snuba PR, sentry CI failed because the field wasn’t being sent to snuba, so snuba crashed. Technically there were multiple problems:
schema validation was missing in some places on producer side
even if the schema was validated on producer side, the field was optional according to it
on the snuba side, the misalignment between consumer and schema was not caught due to the above mypy bug
We should focus on (3) for this ticket. If the field was marked as required in the schema, our existing PR linting would’ve at least told the user that this was a breaking change.
We have a schema definition with two fields, foo_required and foo_optional.
Now consider this code:
VSCode/pyright does error on the above line because it might crash. Unfortunately, mypy does not.
We had an incident (without incident ticket) where this exact crash happened:
c19ad27
This schema change added a new, optional field, but the corresponding Snuba PR accessed the field as if it was required. Mypy did not catch this.
When we merged the Snuba PR, sentry CI failed because the field wasn’t being sent to snuba, so snuba crashed. Technically there were multiple problems:
schema validation was missing in some places on producer side
even if the schema was validated on producer side, the field was optional according to it
on the snuba side, the misalignment between consumer and schema was not caught due to the above mypy bug
We should focus on (3) for this ticket. If the field was marked as required in the schema, our existing PR linting would’ve at least told the user that this was a breaking change.
Proposed solution
see jsonschema-gentypes issue
(also tracked as SNS-2325, see also mypy issue)
The text was updated successfully, but these errors were encountered: