chore(hybrid-cloud): Mark react page view as control test stable #54257
Conversation
github-actions
bot
added
the
Scope: Backend
| organization=organization, slug=kwargs["project_id"] | ||
| ).first() | ||
| first_event_pending.send(project=project, user=request.user, sender=self) | ||
| project = project_service.get_by_slug( |
There was a problem hiding this comment.
It seems like projects already belong to the organization object. I don't think it is necessary to query the project again by slug, a simple filter on the objects inside organization should be sufficient?
| organization_service.schedule_signal( | ||
| signal=first_event_pending, | ||
| organization_id=organization.id, | ||
| args=dict(project=project, user_id=request.user.id if request.user else None), |
There was a problem hiding this comment.
hmmm, I'm not a fan of serializing the entire project object here either. What about just passing the project_id in instead and querying the project on the other side? Should be possible to change the handler.
There was a problem hiding this comment.
I agree with changing the signal handler if we need to make this signal cross-region.
| follow=True, | ||
| ) | ||
| with assume_test_silo_mode(SiloMode.REGION): | ||
| response = self.client.get( |
There was a problem hiding this comment.
We may want to add outbox_runner and receivers_raise_on_send context handlers here to validate and test the actual signal being executed.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #54257 +/- ##
==========================================
+ Coverage 79.68% 79.74% +0.05%
==========================================
Files 4989 4989
Lines 211739 211742 +3
Branches 36090 36093 +3
==========================================
+ Hits 168719 168844 +125
+ Misses 37833 37698 -135
- Partials 5187 5200 +13
|
dashed
force-pushed
the
hybrid-cloud/2-mark-react-page-as-control-stable
branch
from
e8f859e
to
3423cd6
Compare
| ).first() | ||
| first_event_pending.send(project=project, user=request.user, sender=self) | ||
| def handle(self, request: Request, organization: RpcOrganization, **kwargs) -> HttpResponse: | ||
| if "project_slug" in kwargs and "onboarding" in request.GET: |
There was a problem hiding this comment.
Is onboarding ever a falsey value? Previously the logic would not pass for falsey values of onboarding but now it will.
dashed
force-pushed
the
hybrid-cloud/2-mark-react-page-as-control-stable
branch
from
70e3d1a
to
daedb54
Compare
| project = next( | ||
| (p for p in organization.projects if p.slug == kwargs["project_slug"]), None | ||
| ) |
There was a problem hiding this comment.
This looks like it could be expensive. Having RpcOrganization inline all of the projects could be a huge payload as some orgs have 1000s of projects. Could we adapt the signal to pass the project slug instead? The project slug is unlikely to shift under us during onboarding and even if it does the impact of that is very low.
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
first_event_pendingsignal toorganization_service.schedule_signal().SENTRY_USE_SPLIT_DBS=1