Skip to content
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/test-build-docker-image.yml
Original file line number Diff line number Diff line change
Expand Up @@ -499,3 +499,32 @@ jobs:
--header 'Authorization: Bearer ${{ secrets.SLACK_APPSMITH_ALERTS_TOKEN }}' \
--header 'Content-Type: application/json; charset=utf-8' \
--data-raw "$body"


notify-slack-for-pg:
needs: ci-test-result
runs-on: ubuntu-latest

if: ( failure() && github.ref == 'refs/heads/pg' )

steps:
- name: Notify failure on workflow run and on Slack
run: |
set -o errexit
set -o nounset
set -o xtrace

run_url='${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}/attempts/${{ github.run_attempt }}'
slack_message="🚨 TBP workflow failed in <$run_url|${{ vars.EDITION }} attempt ${{ github.run_attempt }} (run ${{ github.run_id }})>."

# This is the ChannelId of the pro-postgres-sync channel.
Comment thread
AnaghHegde marked this conversation as resolved.
Outdated
body="$(jq -nc \
--arg channel C07JMLWEXDJ \
--arg text "$slack_message" \
'$ARGS.named'
)"
curl -v https://slack.com/api/chat.postMessage \
--fail-with-body \
--header 'Authorization: Bearer ${{ secrets.SLACK_APPSMITH_ALERTS_TOKEN }}' \
--header 'Content-Type: application/json; charset=utf-8' \
--data-raw "$body"
Comment on lines +502 to +530

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.

⚠️ Potential issue

Class, let's examine this new addition to our workflow!

Now, pay attention to how we've expanded our communication strategy. We're not just notifying one channel anymore, but two! This is like sending a note home to both your parents and your grandparents.

Let's break it down:

  1. We're using a new channel ID for the "pro-postgres-sync" channel.
  2. We're crafting the message body just like before, very consistent!
  3. We're using curl again to send the message, just like a carrier pigeon but faster!

However, there's room for improvement:

  1. The curl --version command on line 509 seems unnecessary. It's like checking if your pencil works before writing - usually not needed!
  2. We're missing the --fail-with-body option in this new curl command. Remember, it's always good to know why something didn't work!

Let's make these small adjustments:

-          curl --version
           curl -v https://slack.com/api/chat.postMessage \
+            --fail-with-body \
             --header 'Authorization: Bearer ${{ secrets.SLACK_APPSMITH_ALERTS_TOKEN }}' \
             --header 'Content-Type: application/json; charset=utf-8' \
             --data-raw "$body"

Can anyone tell me why these changes will make our code better?

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# This is the ChannelId of the pro-postgres-sync channel.
body="$(jq -nc \
--arg channel C07JMLWEXDJ \
--arg text "$slack_message" \
'$ARGS.named'
)"
curl --version
curl -v https://slack.com/api/chat.postMessage \
--header 'Authorization: Bearer ${{ secrets.SLACK_APPSMITH_ALERTS_TOKEN }}' \
--header 'Content-Type: application/json; charset=utf-8' \
--data-raw "$body"
# This is the ChannelId of the pro-postgres-sync channel.
body="$(jq -nc \
--arg channel C07JMLWEXDJ \
--arg text "$slack_message" \
'$ARGS.named'
)"
curl -v https://slack.com/api/chat.postMessage \
--fail-with-body \
--header 'Authorization: Bearer ${{ secrets.SLACK_APPSMITH_ALERTS_TOKEN }}' \
--header 'Content-Type: application/json; charset=utf-8' \
--data-raw "$body"