Skip to content

chore(webhooks): Include tags by default#110519

Merged
armenzg merged 1 commit into
masterfrom
remove_delivery_time_include_github_tags
Mar 12, 2026
Merged

chore(webhooks): Include tags by default#110519
armenzg merged 1 commit into
masterfrom
remove_delivery_time_include_github_tags

Conversation

@armenzg
Copy link
Copy Markdown
Member

@armenzg armenzg commented Mar 12, 2026

The logic for including GitHub tags has been integrated directly into the _record_delivery_time_metrics function, ensuring consistent behavior without the need for the option in the tests.

…ests

This commit removes the `hybridcloud.deliver_webhooks.delivery_time_include_github_tags` option from the `DeliveryTimeMetricsTest` class, simplifying the tests for delivery time metrics related to GitHub events. The logic for including GitHub tags has been integrated directly into the `_record_delivery_time_metrics` function, ensuring consistent behavior without the need for the option in the tests.
@armenzg armenzg self-assigned this Mar 12, 2026
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 12, 2026
@armenzg armenzg marked this pull request as ready for review March 12, 2026 14:21
@armenzg armenzg requested a review from a team as a code owner March 12, 2026 14:21
@armenzg armenzg requested a review from a team March 12, 2026 14:21
tags = {"region_sent_to": region_sent_to}
if options.get("hybridcloud.deliver_webhooks.delivery_time_include_github_tags"):
tags |= _get_github_delivery_time_tags(payload)
tags = {"region_sent_to": region_sent_to} | _get_github_delivery_time_tags(payload)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[nit] what if we wrapped this call in a try-except in case there's an error that would only show up when this function is called in non control silo context

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I can't see how as this has been running in production without any issues.

@armenzg armenzg merged commit 4d057f3 into master Mar 12, 2026
58 checks passed
@armenzg armenzg deleted the remove_delivery_time_include_github_tags branch March 12, 2026 17:34
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants