chore(sentry_apps): Move Sentry App serializers to sentry_apps! #77995
Conversation
github-actions
bot
added
the
Scope: Backend
❌ 10 Tests Failed:
View the top 3 failed tests by shortest run time
To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard |
| from sentry.models.avatars.sentry_app_avatar import SentryAppAvatar | ||
| from sentry.sentry_apps.api.serializers.sentry_app_requests import SentryAppAvatarSerializer |
There was a problem hiding this comment.
I think it is a good idea to distinguish request and response serializers. Should we have a separate module for request serializers instead of a module suffix? Perhaps something like sentry.sentry_apps.api.parsers for the request parsing serializers?
There was a problem hiding this comment.
that's a good idea. Prob should move the avatar request serializer into its own file while there 🫠
| @@ -17,7 +17,9 @@ class RequestSerializer(Serializer): | |||
| def __init__(self, sentry_app: SentryApp) -> None: | |||
| self.sentry_app = sentry_app | |||
|
|
|||
| def get_attrs(self, item_list: list[Any], user: Any, **kwargs: Any) -> MutableMapping[Any, Any]: | |||
| def get_attrs( | |||
There was a problem hiding this comment.
Neat, we had a response serializer in the rest_framework serializer directory 🫠
There was a problem hiding this comment.
:bufo-melting:
| from sentry.mediators.sentry_app_installations.installation_notifier import InstallationNotifier | ||
| from sentry.mediators.sentry_app_installations.updater import Updater | ||
| from sentry.sentry_apps.api.serializers.parsers.sentry_app_installation import ( |
There was a problem hiding this comment.
Should we have sentry.sentry_apps.api.parsers instead?
| from sentry.auth.staff import is_active_staff | ||
| from sentry.constants import SentryAppStatus | ||
| from sentry.mediators.sentry_app_installations.installation_notifier import InstallationNotifier | ||
| from sentry.organizations.services.organization import organization_service | ||
| from sentry.sentry_apps.api.serializers.parsers.sentry_app import RequestSentryAppSerializer |
There was a problem hiding this comment.
SentryAppParser would be a nice classname to have. Would mirror the Serializer conventions without overlapping terms.
Christinarlong
deleted the
christinarlong/move-sentryapps-serializers
branch
migrate all sentry app related serializers to sentry_apps!
[X] serializers
[X] tests
[x] typing
[] getsentry shim - None so far
issue ref(#73857)