Skip to content

Conversation

@lynnagara
Copy link
Member

note: it is safe to ship frontend + backend together in this case as backend currently works in both silos

note: it is safe to ship frontend + backend together in this case
as backend currently works in both silos
@lynnagara lynnagara requested a review from a team as a code owner December 5, 2025 23:00
@github-actions github-actions bot added Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Dec 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

🚨 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 #discuss-dev-infra channel.

@codecov
Copy link

codecov bot commented Dec 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

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              

@kcons
Copy link
Member

kcons commented Dec 9, 2025

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# TODO(cells): This endpoint is moving to control

Don't need this TODO anymore 😄

@lynnagara
Copy link
Member Author

lynnagara commented Dec 9, 2025

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?

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
the backend was switched from region only -> all silos in a prior PR so i think it's ok for this non-critical API

@markstory
Copy link
Member

so i think it's ok for this non-critical API

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.

@lynnagara lynnagara merged commit 4de2568 into master Dec 9, 2025
70 checks passed
@lynnagara lynnagara deleted the email-capture-demo-mode branch December 9, 2025 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants