Skip to content
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

Integrations: deprecate old incoming webhook URLs #11037

Open
stsewd opened this issue Jan 16, 2024 · 3 comments
Open

Integrations: deprecate old incoming webhook URLs #11037

stsewd opened this issue Jan 16, 2024 · 3 comments
Labels
Needed: design decision A core team decision is required

Comments

@stsewd
Copy link
Member

stsewd commented Jan 16, 2024

What's the problem this feature will solve?

Currently, we have two URLs for incoming webhooks

integration_urls = [
re_path(
r"webhook/github/(?P<project_slug>{project_slug})/$".format(**pattern_opts),
integrations.GitHubWebhookView.as_view(),
name="api_webhook_github",
),
re_path(
r"webhook/gitlab/(?P<project_slug>{project_slug})/$".format(**pattern_opts),
integrations.GitLabWebhookView.as_view(),
name="api_webhook_gitlab",
),
re_path(
r"webhook/bitbucket/(?P<project_slug>{project_slug})/$".format(**pattern_opts),
integrations.BitbucketWebhookView.as_view(),
name="api_webhook_bitbucket",
),
re_path(
r"webhook/generic/(?P<project_slug>{project_slug})/$".format(**pattern_opts),
integrations.APIWebhookView.as_view(),
name="api_webhook_generic",
),
re_path(
(
r"webhook/(?P<project_slug>{project_slug})/"
r"(?P<integration_pk>{integer_pk})/$".format(**pattern_opts)
),
integrations.WebhookView.as_view(),
name="api_webhook",
),
]

One that only takes into consideration the project's slug, which means that only one integration of a type can exist per project.
And the other one take into consideration the project and integration PK, so many integrations of the same type can co-exist.

Looks like we are currently linking to the more specific type of URL for all integrations, any project using the old URL is probably an old project.

Describe the solution you'd like

Just use one URL, ask users to migrate to the more specific URL type.

Maybe it also makes sense to allow only one incoming webhook, not sure when multiple incoming webhooks would be necessary, maybe for debugging only?

Additional context

Currently, if the user makes use of the more specific URL, and has more than one type of that integration, the integration will fail (since we use .get(), and it returns more than one result)

https://read-the-docs.sentry.io/issues/4634041415/?project=148442&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=24h&stream_index=3

@stsewd stsewd added the Needed: design decision A core team decision is required label Jan 16, 2024
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Jan 16, 2024
@humitos
Copy link
Member

humitos commented Jan 17, 2024

I think these two points are key to decide before moving forward.

Just use one URL, ask users to migrate to the more specific URL type.

Is it possible to remove any "manual action from the user" from the equation? Can we migrate them automatically?

How many of these old integrations we have? How many of them are from "active projects"?

Maybe it also makes sense to allow only one incoming webhook, not sure when multiple incoming webhooks would be necessary, maybe for debugging only?

How many projects we have with multiple incoming webhooks? What are those?

I think that knowing how they are configured, we can infer why they need more than one integration. Maybe it's just unnecessary. In that case, I'd vote to remove this extra complexity from our code, but also simplify the UX.

@humitos
Copy link
Member

humitos commented Jan 17, 2024

Related to #10760

@agjohnson
Copy link
Contributor

Needed here:

  • Get number of users hitting these URLs
  • Plan a deprecation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: design decision A core team decision is required
Projects
Status: Planned
Development

No branches or pull requests

3 participants