feat: enable updating webhook events for published integration
This PR enables updating events for published apps. There is a bug where we would prevent updating the webhook events because the scopes were out of order which actually turned out to be a blessing in disguise because the current implementation for updating the service hooks was not correct for published integrations which are installed by multiple integrations. As it happens, for that to work we have to update every ServiceHook row that is attached to that integration. Because there could be thousands of them, I put that logic in a task because it would almost certainly time out if there are many installations.
Note I still have to update the UI next so we allow updating the installation; right now the UI will prevent the update.
🚨 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.
@markstory I think there is some weird hybrid cloud shenanigans going on in this test which is causing it to fail. I added some print statements for the failure:
Service hook rpc: added hook for organization 4554523754102784 and application 76
Service hook rpc: added hook for organization 4554523754168325 and application 73
Service hook rpc: added hook for organization 4554523754233856 and application 73
start test with organization ids: 4554523754168325 4554523754233856 and application id: 73
running create_or_update_service_hooks_for_installation with application id 73
running create_or_update_service_hooks_for_installation with application id 73
task and outbox runner should be done by now with organization ids: 4554523754168325, 4554523754233856, application id: 73
Which has this test failure:
AssertionError: No service hooks found for Org1 (ID: 4554523754168325), App ID: 73
There are a number of things going on that don't make sense:
- The print statements for the service hooks being updated are happening before the API call starts
- There is an extraneous application and organization that's getting a service hook update
- There are print statements that indeed the service hooks were created before we did the assertion
Here is the link to the test: https://github.com/getsentry/sentry/pull/75903/files#diff-253b2368a9e5befb9458c117ee39b0dda4a30afbd997dd320d0aff1d01ebb051R276
Any idea what's going on? I'm really out of clues and I can't replicate this locally. But I'm pretty sure this has to do with Hybrid Cloud because the project service hooks exist in the region silo while this is a control silo test.
@corps Maybe you have some idea about my last comment?
I was not able to reproduce the failure when running tests in isolation. However, I was able to reproduce the problem by running the same slice of CI with
TEST_GROUP=2 TOTAL_TEST_GROUPS=11 MATRIX_INSTANCE_TOTAL=11 pytest tests --ignore tests/acceptance --ignore tests/apidocs --ignore tests/js --ignore tests/tools -x
1. The print statements for the service hooks being updated are happening before the API call starts 2. There is an extraneous application and organization that's getting a service hook update 3. There are print statements that indeed the service hooks were created before we did the assertion
I don't know, but I suspect given all three things, it has to be test pollution somehow. Given the log outputs, it makes me think, is it possible that a celery job is getting kicked off, not caught in tasks context manager, and is run during another test?