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

chore(slack): remove old client from webhook handling code #74104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cathteng
Copy link
Member

Remove the old slack client completely from Slack code that handles updating a group, specifically removing from the code the processes updates from the archive and resolve modals (and updates the Slack message).

Also adds metrics for all of the group update code in Slack, we also have another place in the same function that updates the Slack message for group updates not through the modal (e.g. mark the issue as ongoing)

@cathteng cathteng requested a review from a team July 10, 2024 21:40
@cathteng cathteng requested a review from a team as a code owner July 10, 2024 21:40
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 10, 2024
Comment on lines +673 to +679
logger.info(
"slack.webhook.view_submission.success",
extra={
"integration_id": slack_request.integration.id,
"blocks": json_blocks,
},
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the success log now that we're moving to enabled? Plus we have the metrics for it to monitor

tags={"type": "submit_modal"},
)
logger.error(
"slack.webhook.view_submission.response-error", extra={"error": str(e)}
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to log more information to help with debugging?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants