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

Send Slack message CI step failure #3781

Closed
obulat opened this issue Feb 9, 2024 · 3 comments · Fixed by #3851
Closed

Send Slack message CI step failure #3781

obulat opened this issue Feb 9, 2024 · 3 comments · Fixed by #3851
Assignees
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: mgmt Related to repo management and automations

Comments

@obulat
Copy link
Contributor

obulat commented Feb 9, 2024

Description

When the PR has quotes in it, the Send Slack message CI step fails because of invalid JSON. Changing the title and re-running does not fix the problem because the old invalid title is re-used.

Reproduction

See https://github.com/WordPress/openverse/actions/runs/7841858042/job/21399038852?pr=3779

Additional context

Medium priority because it affects the CI status for PRs, but does not prevent us from merging them (the check is not mandatory).

@obulat obulat added 🟨 priority: medium Not blocking but should be addressed soon 🛠 goal: fix Bug fix 🤖 aspect: dx Concerns developers' experience with the codebase 🧱 stack: mgmt Related to repo management and automations labels Feb 9, 2024
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Feb 9, 2024
@AetherUnbound
Copy link
Collaborator

That's frustrating, #3705 was supposed to address this 😞

@dhruvkb
Copy link
Member

dhruvkb commented Feb 12, 2024

I think the safest approach here would be to have a Bash (or Python) step before the Slack steps that sanitises the PR title from the env var and writes back to the env var. We could also just strip away any quotes in the title altogether, which will lead to slightly incorrect but still readable Slack notifications.

Does this^ seem like a solution worth exploring?

@AetherUnbound
Copy link
Collaborator

That sounds like a good approach Dhruv! Trying to figure out the appropriate escaping for this is quite the headache.

@dhruvkb dhruvkb self-assigned this Feb 29, 2024
@openverse-bot openverse-bot moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Feb 29, 2024
@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Feb 29, 2024
@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: mgmt Related to repo management and automations
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants