-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add rudimentary scheduled check of dev container #10
base: main
Are you sure you want to change the base?
Conversation
.github/workflows/test.yml
Outdated
# - name: Notify Slack on failure | ||
# if: failure() && github.event_name == 'schedule' | ||
# uses: zuplo/github-action-slack-notify-build@cf8e7e66a21d76a8125ea9648979c30920195552 # v2 | ||
# env: | ||
# SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} | ||
# with: | ||
# channel_id: <TODO: add a channel id> | ||
# status: "Scheduled dev container test run failure" | ||
# color: danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: A Slack notification is probably best for this kind of failure. This needs the channel choosing, and this section then being uncommented.
Without Slack, GitHub notifications of failures of scheduled runs will only be sent to the user that last edited the cron
line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's a #team-RAP owned repository then that's where it should be sent?
All this is does is run the existing ehrQL project with `opensafely run`. That's probably sufficient for now. This needs the channel ID deciding upon before notifying Slack.
f2e201a
to
763efe1
Compare
We've added a workflow, so it's a good idea to be notified of updates to actions.
763efe1
to
cefb43b
Compare
if: failure() && github.event_name == 'schedule' | ||
uses: zuplo/github-action-slack-notify-build@cf8e7e66a21d76a8125ea9648979c30920195552 # v2 | ||
env: | ||
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: This repository will need to get access to this secret, if this PR is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I think an 🦩 should be able to grant access to the org secret for this right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's right.
This was just a reminder if someone merges this PR and doesn't realise it.
env: | ||
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} | ||
with: | ||
channel_id: C069YDR4NCA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This is the #team-rap
channel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they own this, then that seems right. Although if #team-rap
own this repo then why are we adding scheduled tests to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I asked the RAP team in Slack if they thought this PR useful or not. We can leave it up to them to decide.
If it's not thought to be useful, it's fine by me to close it without merging, of course.
Fixes #9.
#6 was reported by a user. It would be preferable to catch such breakage ourselves; running a scheduled check could help there.