chore(cells) Finish adding silo annotations to View, Endpoint and functions#104172
chore(cells) Finish adding silo annotations to View, Endpoint and functions#104172
Conversation
Add silo annotations to the remaining view classes that were missing them. Refs INFRENG-202
These aren't strictly needed but it makes test validation simpler
|
|
||
| # Relocation variation of password recovery | ||
| relocate_confirm = partial(recover_confirm, mode="relocate") | ||
| relocate_confirm = control_silo_view(partial(recover_confirm, mode="relocate")) |
There was a problem hiding this comment.
Bug: Double silo decoration on partials causes redundant checks
The recover_confirm function is decorated with @control_silo_view, and then set_password_confirm and relocate_confirm wrap partial(recover_confirm, ...) with another control_silo_view(). Since the partial wraps the already-decorated function, this results in double decoration. When either partial is called, the silo mode check runs twice: once on the outer wrapper and once when the partial invokes the inner decorated recover_confirm. The fix would be to either remove the decorator from recover_confirm (if it's only accessed via partials) or not wrap the partials (and manually set the silo_limit attribute to satisfy the test).
Additional Locations (1)
There was a problem hiding this comment.
The redundancy is a cost I'm willing to accept for being able to write more thorough tests.
| and view_class != RedirectView | ||
| and view_class != TemplateView |
There was a problem hiding this comment.
Is this an extensive list of view classes we want to exclude from this? Should we consider making this a const list somewhere instead?
There was a problem hiding this comment.
So far, these are the only django base views we're using. If we get any more of them a list would be better.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
we hit these often in tests with region mode.
With some more all_silo_views, the URL generation needed to be changed. We only need URL patterns for endpoints that are *only* on control.
|
🚨 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 |
Refs INFRENG-202