-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(cells): make email capture demo mode control-silo only #104488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
note: it is safe to ship frontend + backend together in this case as backend currently works in both silos
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #104488 +/- ##
===========================================
+ Coverage 80.53% 80.54% +0.01%
===========================================
Files 9350 9351 +1
Lines 400099 400614 +515
Branches 25660 25660
===========================================
+ Hits 322216 322687 +471
- Misses 77415 77459 +44
Partials 468 468 |
|
For my education, why is this safe? It'd seem that this restricts the endpoint to control and thus old UI code would be potentially broken until a new UI push, so we'd want to make sure the UI change goes out before the backend. Do we not limit by silo in the API this way? Or is "ui deploys first" a safe enough bet that it's not worth worrying about? |
| email = serializers.EmailField(required=True) | ||
|
|
||
|
|
||
| # TODO(cells): This endpoint is moving to control |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # TODO(cells): This endpoint is moving to control |
Don't need this TODO anymore 😄
oh yeah you're right.. this does assume the UI goes out first. i think it's ok in this case -- the frontend control routes are generated from the backend change so it's kinda designed to go out together |
This was my thinking as well. The demo email intake is low enough that a deploy cycle will be fine here as the frontend will deploy well before the API servers will. |
note: it is safe to ship frontend + backend together in this case as backend currently works in both silos