ref(sentry-app): Allows default serializer to take a human readable list of features instead of int values #83200
Conversation
…ist of features instead of int values
github-actions
bot
added
the
Scope: Backend
| API = 0, "integrations-api" | ||
| ISSUE_LINK = 1, "integrations-issue-link" | ||
| STACKTRACE_LINK = 2, "integrations-stacktrace-link" | ||
| EVENT_HOOKS = ( | ||
| 3, | ||
| "integrations-event-hooks", | ||
| ) | ||
| PROJECT_MANAGEMENT = 4, "integrations-project-management" | ||
| INCIDENT_MANAGEMENT = 5, "integrations-incident-management" | ||
| FEATURE_FLAG = 6, "integrations-feature-flag" | ||
| ALERTS = 7, "integrations-alert-rule" | ||
| RELEASE_MANAGEMENT = 8, "integrations-release-management" | ||
| VISUALIZATION = 9, "integrations-visualization" | ||
| CHAT = 11, "integrations-chat" | ||
| SESSION_REPLAY = 12, "integrations-session-replay" | ||
|
|
||
| feature_name: str | ||
|
|
||
| def __new__(cls, value: int, feature_name: str) -> Feature: | ||
| obj = int.__new__(cls, value) | ||
| obj._value_ = value | ||
| obj.feature_name = feature_name |
There was a problem hiding this comment.
This implementation allows us to specify enums with both int and string values. Overriding __new__ facilitates constructing an int object, with enum values attached in a similar fashion as Python's std lib implementation for HTTP status codes: https://github.com/python/cpython/blob/main/Lib/http/__init__.py
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #83200 +/- ##
==========================================
+ Coverage 87.57% 87.59% +0.01%
==========================================
Files 9469 9369 -100
Lines 536960 536430 -530
Branches 21119 21062 -57
==========================================
- Hits 470249 469890 -359
+ Misses 66352 66181 -171
Partials 359 359 |
| feature_list = result.get("features") | ||
| if feature_set := result.get("feature_set"): |
There was a problem hiding this comment.
Should we be checking for equality of these 2 ?
EDIT: oh wait nvm, it seems like there can only be one ?
There was a problem hiding this comment.
Yep! I made the options mutually exclusive. There are more edge cases handling 2 than 1.
| @@ -73,7 +73,10 @@ class SentryAppParser(Serializer): | |||
| status = serializers.CharField(required=False, allow_null=True) | |||
| events = EventListField(required=False, allow_null=True) | |||
| features = serializers.MultipleChoiceField( | |||
There was a problem hiding this comment.
will we deprecate this eventually?
There was a problem hiding this comment.
Yep, once the UI is migrated and working correctly, I'll drop this field
|
We're re-evaluating this patch entirely, closing for now but will reopen if we decide to publicize this functionality. |
Updates the default
SentryAppParserclass (serializer) to include a new field namedfeature_set, which will supersedefeatureseventually in our Sentry App APIs.The
featuresfield has a couple of key problems:By allowing our API users to specify
feature_setwith the string format of the names, this should address both of the above issues allowing us to fully publish the API.To do this, I changed the implementation of the
Featureclass to include string representations of each int enum, by using a pattern from Python's std lib code for HTTP status enums: https://github.com/python/cpython/blob/main/Lib/http/__init__.pyThe alternative would have been to create some sort of map of the hardcoded values in the original class, but this felt like a cleaner solution.
Note:
feature_setandfeaturesare mutually exclusive fields which will raise validation errors if both are included in aPUTrequest body.