From 031a447f16d388a8d7766c588b09ac44dcb095be Mon Sep 17 00:00:00 2001 From: Sam Winebrake <85908068+samwinebrake@users.noreply.github.com> Date: Sun, 16 Jun 2024 15:02:31 -0400 Subject: [PATCH] Reusable notification workflow (#665) * Create user_notification_system.yml * add email function to helpers * add failure notification when checks don't pass * Update score_new_plugins.yml * switch from Failure to False * change failure to false * add failure label after email notification sent * [still testing] notified failure check and removal * addition and check of failure-notified label * change to GITHUB_TOKEN --------- Co-authored-by: Martin Schrimpf --- .../check_if_pr_is_automergeable.yml | 135 ++++++++++++++-- .github/workflows/score_new_plugins.yml | 102 ++++++++---- .../workflows/user_notification_system.yml | 146 ++++++++++++++++++ .../submission/actions_helpers.py | 28 ++++ 4 files changed, 369 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/user_notification_system.yml diff --git a/.github/workflows/check_if_pr_is_automergeable.yml b/.github/workflows/check_if_pr_is_automergeable.yml index 0669eeed9..4cc627632 100644 --- a/.github/workflows/check_if_pr_is_automergeable.yml +++ b/.github/workflows/check_if_pr_is_automergeable.yml @@ -1,39 +1,111 @@ name: Check if PR is automergeable -# Triggered on all PRs either by +# Triggered on all PRs by # - completion of CI checks (status events), OR -# - tagging with "automerge" or "automerge-web" labels +# - tagging with "automerge" or "automerge-web" labels, OR +# - updates to current PRs +# # This workflow checks if the PR that invoked the trigger is automergeable. # A PR is automergeable iff it: -# 1) is labeled "automerge" OR "automerge-web" (originates from web submission) +# 1) is labeled "automerge" OR "automerge-web" (originates from web submission) (checked in actions_helpers.py) # 2) only changes plugins (subdirs of /benchmarks, /data, /models, /metrics) # 3) passes all tests (Travis and Jenkins). # If all 3 conditions are met, the "automerge-approved" label is applied to the PR # (This label triggers the `automerge_plugin-only_prs` workflow to merge the PR.) +# +# If any test fails, the user will be notified by the brain-score email account. +# If the user has already been notified of a test failure and there have been no pushes to the PR, no email will be sent. on: pull_request: - types: [labeled] + types: [labeled, synchronize] status: permissions: write-all jobs: + check_trigger: + name: Check what triggered this workflow. If it was the addition of a 'failure-notified' label, skip the rest of workflow. + runs-on: ubuntu-latest + outputs: + PROCEED: ${{ steps.check_label.outputs.PROCEED }} + steps: + - name: Check trigger condition + id: check_label + run: | + if [[ "${{ github.event_name }}" == "pull_request" && "${{ github.event.action }}" == "labeled" ]]; then + LABEL_NAME="${{ github.event.label.name }}" + echo "Trigger label: $LABEL_NAME" + if [[ "$LABEL_NAME" == "failure-notified" ]]; then + echo "PROCEED=false" >> $GITHUB_OUTPUT + else + echo "PROCEED=true" >> $GITHUB_OUTPUT + fi + else + echo "PROCEED=true" >> $GITHUB_OUTPUT + fi + + remove_failure_notified_label: + name: On new push, remove the 'failure-notified' label. + if: ${{ (github.event_name == 'pull_request') && (github.event.action == 'synchronize') }} + runs-on: ubuntu-latest + steps: + - name: Remove 'failure-notified' label on new push to PR + uses: actions/github-script@v6 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const prNumber = context.payload.pull_request.number; + const { data: labels } = await github.rest.issues.listLabelsOnIssue({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber + }); + if (labels.find(label => label.name === 'failure-notified')) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + name: 'failure-notified' + }); + } - check_test_results: - name: Check if all tests have passed and PR meets automerge conditions + check_pr_details: + name: Check all details of the PR (if all tests have passed, PR meets automerge conditions, pr number, label is 'automerge-web') runs-on: ubuntu-latest + needs: check_trigger + if: needs.check_trigger.outputs.PROCEED == 'true' outputs: - ALL_TESTS_PASS: ${{ steps.gettestresults.outputs.TEST_RESULTS }} + ALL_TESTS_PASS: ${{ steps.get_test_results.outputs.TEST_RESULTS }} + PR_NUMBER: ${{ steps.get_pr_number.outputs.PR_NUMBER }} + AUTOMERGE_WEB: ${{ steps.check_automerge_web_label.outputs.AUTOMERGE_WEB }} steps: - name: Check out repository code uses: actions/checkout@v4 with: fetch-depth: 0 + - name: Get PR number from workflow context + id: get_pr_number + run: | + echo "PR_NUMBER=$( python brainscore_vision/submission/actions_helpers.py get_pr_num )" >> $GITHUB_OUTPUT + - name: Check if PR has 'automerge-web' label + id: check_automerge_web_label + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + LABELS_JSON=$(gh pr view ${{ steps.get_pr_number.outputs.PR_NUMBER }} --json labels) + echo "Labels: $LABELS_JSON" + if echo "$LABELS_JSON" | jq -e '.labels[] | select(.name == "automerge-web")' >/dev/null; then + echo "Found automerge-web label." + echo "AUTOMERGE_WEB=true" >> $GITHUB_OUTPUT + else + echo "automerge-web label not found." + echo "AUTOMERGE_WEB=false" >> $GITHUB_OUTPUT + fi - name: Get test results and ensure automergeable - id: gettestresults + id: get_test_results run: | echo "Checking test results for PR head $( python brainscore_vision/submission/actions_helpers.py get_pr_head )" test_results=$( python brainscore_vision/submission/actions_helpers.py ) @@ -45,20 +117,55 @@ jobs: runs-on: ubuntu-latest permissions: issues: write - needs: check_test_results - if: needs.check_test_results.outputs.ALL_TESTS_PASS == 'True' + needs: [check_pr_details, check_trigger] + if: needs.check_pr_details.outputs.ALL_TESTS_PASS == 'True' && needs.check_trigger.outputs.PROCEED == 'true' steps: - name: Check out repository code uses: actions/checkout@v4 with: fetch-depth: 0 - - name: Get PR number from workflow context - run: | - echo "PR_NUMBER=$( python brainscore_vision/submission/actions_helpers.py get_pr_num )" >> $GITHUB_ENV - name: Add automerge-approved label to PR env: GH_TOKEN: ${{ secrets.WORKFLOW_TOKEN }} GH_REPO: ${{ github.repository }} - NUMBER: ${{ env.PR_NUMBER }} + NUMBER: ${{ needs.check_pr_details.outputs.PR_NUMBER }} LABELS: automerge-approved run: gh issue edit "$NUMBER" --add-label "$LABELS" + + check_email_label: + name: Check if user has already been notified of failure + runs-on: ubuntu-latest + needs: [check_pr_details, check_trigger] + outputs: + FAILURE_NOTIFIED: ${{ steps.check_failure_notified_label.outputs.FAILURE_NOTIFIED }} + if: needs.check_pr_details.outputs.ALL_TESTS_PASS == 'False' && needs.check_pr_details.outputs.AUTOMERGE_WEB == 'true' && needs.check_trigger.outputs.PROCEED == 'true' + steps: + - name: Check out repository code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Check if PR has 'failure-notified' label already + id: check_failure_notified_label + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + LABELS_JSON=$(gh pr view ${{ needs.check_pr_details.outputs.PR_NUMBER }} --json labels) + echo "Labels: $LABELS_JSON" + if echo "$LABELS_JSON" | jq -e '.labels[] | select(.name == "failure-notified")' >/dev/null; then + echo "Found failure-notified label." + echo "FAILURE_NOTIFIED=true" >> $GITHUB_OUTPUT + else + echo "failure-notified label not found." + echo "FAILURE_NOTIFIED=false" >> $GITHUB_OUTPUT + fi + + notify_failure: + name: If any test fails and failure hasn't been notified, notify the user through the brain-score email account (only needed for web submissions) + uses: ./.github/workflows/user_notification_system.yml + needs: [check_pr_details, check_email_label, check_trigger] + if: needs.check_pr_details.outputs.ALL_TESTS_PASS == 'False' && needs.check_pr_details.outputs.AUTOMERGE_WEB == 'true' && (needs.check_email_label.outputs.FAILURE_NOTIFIED == 'false') && (needs.check_trigger.outputs.PROCEED == 'true') + with: + pr_number: ${{ needs.check_pr_details.outputs.PR_NUMBER }} + is_automerge_web: true + action_type: 'send_email' + secrets: inherit diff --git a/.github/workflows/score_new_plugins.yml b/.github/workflows/score_new_plugins.yml index f8d55d060..8e4c8aec9 100644 --- a/.github/workflows/score_new_plugins.yml +++ b/.github/workflows/score_new_plugins.yml @@ -64,30 +64,27 @@ jobs: id: scoringneeded run: | echo "RUN_SCORING=$(jq -r '.run_score' <<< ${{ steps.getpluginfo.outputs.PLUGIN_INFO }})" >> $GITHUB_OUTPUT - - - name: Find PR author email for non-web submissions - if: "!contains(github.event.pull_request.labels.*.name, 'automerge-web') && steps.scoringneeded.outputs.RUN_SCORING == 'True'" - uses: evvanErb/get-github-email-by-username-action@v2.0 - id: getemail - with: - github-username: ${{github.event.pull_request.user.login}} - token: ${{ secrets.GITHUB_TOKEN }} # Including token enables most reliable way to get a user's email - - name: Update PLUGIN_INFO for non-web submissions - if: "!contains(github.event.pull_request.labels.*.name, 'automerge-web') && steps.scoringneeded.outputs.RUN_SCORING == 'True'" - id: non_automerge_web + + - name: Check for automerge-web label + id: check_label run: | - echo "The PR author email is ${{ steps.getemail.outputs.email }}" - echo "PLUGIN_INFO=$(<<<${{ steps.getpluginfo.outputs.PLUGIN_INFO }} tr -d "'" | jq -c '. + {email: "${{ steps.getemail.outputs.email }}"}')" >> $GITHUB_ENV - - - name: Update PLUGIN_INFO for automerge-web (find uid, public v. private, and bs email) - if: contains(github.event.pull_request.labels.*.name, 'automerge-web') && steps.scoringneeded.outputs.RUN_SCORING == 'True' - id: automerge_web + LABELS_JSON="${{ toJSON(github.event.pull_request.labels.*.name) }}" + if echo "$LABELS_JSON" | grep -q "automerge-web"; then + echo "has_automerge_web=true" >> $GITHUB_ENV + else + echo "has_automerge_web=false" >> $GITHUB_ENV + fi + + - name: Update PLUGIN_INFO based on label run: | - BS_UID="$(echo '${{ github.event.pull_request.title }}' | sed -E 's/.*\(user:([^)]+)\).*/\1/')" - BS_PUBLIC="$(echo '${{ github.event.pull_request.title }}' | sed -E 's/.*\(public:([^)]+)\).*/\1/')" - USER_EMAIL=$(python -c "from brainscore_core.submission.database import email_from_uid; from brainscore_core.submission.endpoints import UserManager; user_manager=UserManager(db_secret='${{ secrets.BSC_DATABASESECRET }}'); print(email_from_uid($BS_UID))") - echo "::add-mask::$USER_EMAIL" # Mask the USER_EMAIL - echo "PLUGIN_INFO=$(<<<${{ steps.getpluginfo.outputs.PLUGIN_INFO }} tr -d "'" | jq -c ". + {user_id: \"$BS_UID\", public: \"$BS_PUBLIC\", email: \"$USER_EMAIL\"}")" >> $GITHUB_ENV + if [[ "$has_automerge_web" == "true" ]]; then + BS_UID="$(echo '${{ github.event.pull_request.title }}' | sed -E 's/.*\(user:([^)]+)\).*/\1/')" + BS_PUBLIC="$(echo '${{ github.event.pull_request.title }}' | sed -E 's/.*\(public:([^)]+)\).*/\1/')" + PLUGIN_INFO=$(echo ${{ steps.getpluginfo.outputs.PLUGIN_INFO }} | tr -d "'" | jq -c ". + {user_id: \"$BS_UID\", public: \"$BS_PUBLIC\"}") + echo "PLUGIN_INFO=${PLUGIN_INFO}" >> $GITHUB_ENV + else + echo "PLUGIN_INFO=$(echo ${{ steps.getpluginfo.outputs.PLUGIN_INFO }} | tr -d "'")" >> $GITHUB_ENV + fi - name: Write PLUGIN_INFO to a json file run: | @@ -98,12 +95,64 @@ jobs: with: name: plugin-info path: plugin-info.json + + extract_email: + name: Extracts email for both PRs and web submissions + uses: ./.github/workflows/user_notification_system.yml + needs: process_submission + if: ${{ needs.process_submission.outputs.RUN_SCORING == 'True' }} + with: + pr_username: ${{github.event.pull_request.user.login}} + pr_title: ${{ github.event.pull_request.title }} + is_automerge_web: ${{ contains(github.event.pull_request.labels.*.name, 'automerge-web') }} + action_type: 'extract_email' + secrets: inherit + + update_plugin_info: + name: Updates PLUGIN_INFO with various fields (domain, competition, model_type, email) + runs-on: ubuntu-latest + needs: extract_email + steps: + - name: Download PLUGIN_INFO artifact + uses: actions/download-artifact@v2 + with: + name: plugin-info + path: artifact-directory + + - name: Set PLUGIN_INFO as an environment variable + run: | + PLUGIN_INFO=$(cat artifact-directory/plugin-info.json) + echo "PLUGIN_INFO=${PLUGIN_INFO}" >> $GITHUB_ENV + - name: Decrypt and mask user email + run: | + DECRYPTED_EMAIL=$(echo "${{ needs.extract_email.outputs.extracted_email }}" | openssl enc -aes-256-cbc -a -d -salt -pass pass:${{ secrets.EMAIL_ENCRYPTION_KEY }}) + echo "::add-mask::$DECRYPTED_EMAIL" + echo "USER_EMAIL=${DECRYPTED_EMAIL}" >> $GITHUB_ENV + + - name: Update PLUGIN_INFO + run: | + PLUGIN_JSON=$(echo "$PLUGIN_INFO" | jq -c '. + {domain: "vision", competition: "None", model_type: "Brain_Model"}') + echo "PLUGIN_INFO=$PLUGIN_JSON" >> $GITHUB_ENV + + PLUGIN_JSON=$(echo "$PLUGIN_JSON" | jq -c --arg email "$USER_EMAIL" '. + {email: $email}') + echo "PLUGIN_INFO=$PLUGIN_JSON" >> $GITHUB_ENV + echo "Updated PLUGIN_INFO: $PLUGIN_JSON" + + - name: Write PLUGIN_INFO to a json file + run: | + echo "$PLUGIN_INFO" > plugin-info.json + + - name: Upload PLUGIN_INFO as an artifact + uses: actions/upload-artifact@v2 + with: + name: plugin-info + path: plugin-info.json run_scoring: name: Score plugins runs-on: ubuntu-latest - needs: [process_submission] + needs: [process_submission, extract_email, update_plugin_info] if: needs.process_submission.outputs.RUN_SCORING == 'True' env: JENKINS_USER: ${{ secrets.JENKINS_USER }} @@ -121,12 +170,8 @@ jobs: run: | PLUGIN_INFO=$(cat artifact-directory/plugin-info.json) USER_EMAIL=$(echo "$PLUGIN_INFO" | jq -r '.email') - echo "::add-mask::$USER_EMAIL" # add a mask when bringing email back from artifact + echo "::add-mask::$USER_EMAIL" # readd a mask when bringing email back from artifact echo "PLUGIN_INFO=${PLUGIN_INFO}" >> $GITHUB_ENV - - - name: Add domain, public, competition, and model_type to PLUGIN_INFO - run: | - echo "PLUGIN_INFO=$(<<<$PLUGIN_INFO tr -d "'" | jq -c '. + {domain: "vision", competition: "None", model_type: "Brain_Model"}')" >> $GITHUB_ENV - name: Check out repository code uses: actions/checkout@v4 @@ -144,3 +189,4 @@ jobs: - name: Run scoring run: | python -c 'from brainscore_core.submission.endpoints import call_jenkins; call_jenkins('\''${{ env.PLUGIN_INFO }}'\'')' + diff --git a/.github/workflows/user_notification_system.yml b/.github/workflows/user_notification_system.yml new file mode 100644 index 000000000..9701bf37d --- /dev/null +++ b/.github/workflows/user_notification_system.yml @@ -0,0 +1,146 @@ +name: User notification system + + +# Triggered by the 'check_if_pr_is_automereable.yml' and 'score_new_plugins.yml' workflows +# This workflow has two distinct purposes: +# - extracting an email address (either from a web submission, or from a PR) +# - sending a PR failure email + +on: + workflow_call: + inputs: + pr_number: + required: false + type: string + pr_username: + required: false + type: string + pr_title: + required: false + type: string + is_automerge_web: + required: true + type: boolean + action_type: + required: true + type: string + description: 'Determines the action to take, e.g., "extract_email" or "send_email".' + outputs: + extracted_email: + description: 'The extracted email address.' + value: ${{ jobs.extract_email.outputs.email }} + +permissions: write-all + +jobs: + extract_email: + name: Extract user email + runs-on: ubuntu-latest + outputs: + email: ${{ steps.set_email_output.outputs.EMAIL }} + steps: + - name: Check out repository code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python 3.7 + uses: actions/setup-python@v4 + with: + python-version: 3.7 + + - name: Configure AWS Credentials + uses: aws-actions/configure-aws-credentials@v1 + with: + aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} + aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + aws-region: us-east-1 + + - name: Installing package dependencies + run: | + python -m pip install --upgrade pip setuptools + python -m pip install "." + + - name: Find PR author email for non-web submissions + if: ${{ !inputs.is_automerge_web }} + uses: evvanErb/get-github-email-by-username-action@v2.0 + id: getemail + with: + github-username: ${{inputs.pr_username}} + token: ${{ secrets.GITHUB_TOKEN }} # Including token enables most reliable way to get a user's email + + - name: Update email for non-web submissions + if: ${{ !inputs.is_automerge_web }} + id: non_automerge_web + run: | + EMAIL=${{ steps.getemail.outputs.email }} + echo "::add-mask::$EMAIL" # Mask the EMAIL + echo "EMAIL=$EMAIL" >> $GITHUB_ENV + + - name: Check if pr title provided + if: inputs.is_automerge_web + id: get_pr_title + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + if [ -z "${{ inputs.pr_title }}" ]; then + echo "Fetching PR title because it wasn't provided" + PR_TITLE=$(gh pr view ${{ inputs.pr_number }} --repo ${{ github.repository }} --json title -q .title) + echo "PR_TITLE=$PR_TITLE" >> $GITHUB_ENV + else + echo "PR_TITLE=${{ inputs.pr_title }}" >> $GITHUB_ENV + fi + + - name: Update email for automerge-web (find email from uid) + if: inputs.is_automerge_web + id: automerge_web + run: | + BS_UID="$(echo $PR_TITLE | sed -E 's/.*\(user:([^)]+)\).*/\1/')" + EMAIL=$(python -c "from brainscore_core.submission.database import email_from_uid; from brainscore_core.submission.endpoints import UserManager; user_manager=UserManager(db_secret='${{ secrets.BSC_DATABASESECRET }}'); print(email_from_uid($BS_UID))") + echo "::add-mask::$EMAIL" # Mask the EMAIL + echo "EMAIL=$EMAIL" >> $GITHUB_ENV + + - name: Encrypt and set job-level output for email + id: set_email_output + run: | + ENCRYPTED_EMAIL=$(echo -n $EMAIL | openssl enc -aes-256-cbc -a -salt -pass pass:${{ secrets.EMAIL_ENCRYPTION_KEY }}) + echo "EMAIL=$ENCRYPTED_EMAIL" >> $GITHUB_OUTPUT + + - name: Write email to file + if: inputs.action_type == 'send_email' + run: echo "$EMAIL" > email.txt + + - name: Upload email as artifact + if: inputs.action_type == 'send_email' + uses: actions/upload-artifact@v2 + with: + name: email-artifact + path: email.txt + + notify_user: + name: Notify user of failure # only necessary for automerge_web labeled since github auto sends email on failure otherwise + runs-on: ubuntu-latest + needs: extract_email + if: inputs.action_type == 'send_email' + steps: + - name: Check out repository code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Download email artifact + uses: actions/download-artifact@v2 + with: + name: email-artifact + + - name: Send email notification + run: | + python brainscore_vision/submission/actions_helpers.py send_failure_email $(cat email.txt) ${{ inputs.pr_number }} ${{ secrets.GMAIL_USERNAME }} ${{ secrets.GMAIL_PASSWORD }} + + - name: Add failure-notified label to PR + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + NUMBER: ${{ inputs.pr_number }} + LABELS: failure-notified + run: gh issue edit "$NUMBER" --add-label "$LABELS" diff --git a/brainscore_vision/submission/actions_helpers.py b/brainscore_vision/submission/actions_helpers.py index 0fa92f6c7..6593cabd9 100644 --- a/brainscore_vision/submission/actions_helpers.py +++ b/brainscore_vision/submission/actions_helpers.py @@ -13,7 +13,9 @@ import os import requests import sys +import smtplib from typing import Union +from email.mime.text import MIMEText BASE_URL = "https://api.github.com/repos/brain-score/vision" @@ -85,12 +87,34 @@ def are_all_tests_passing(test_results: dict) -> dict: return False else: return True + +def any_tests_failing(test_results: dict) -> dict: + if any(result == "failure" for result in test_results.values()): + return True + else: + return False def is_labeled_automerge(pr_num: int) -> bool: label_data = get_data(f"{BASE_URL}/issues/{pr_num}/labels") labeled_automerge = any(label['name'] in ('automerge', 'automerge-web') for label in label_data) return labeled_automerge +def send_failure_email(email: str, pr_number: str, mail_username: str, mail_password: str): + """ Send submitter an email if their web-submitted PR fails. """ + body = "Your Brain-Score submission did not pass checks. " \ + "Please review the test results and update the PR at " \ + f"https://github.com/brain-score/vision/pull/{pr_number} " \ + "or send in an updated submission via the website." + msg = MIMEText(body) + msg['Subject'] = "Brain-Score submission failed" + msg['From'] = "Brain-Score" + msg['To'] = email + + # send email + with smtplib.SMTP_SSL('smtp.gmail.com', 465) as smtp_server: + smtp_server.login(mail_username, mail_password) + smtp_server.sendmail(mail_username, email, msg.as_string()) + if __name__ == "__main__": @@ -115,6 +139,7 @@ def is_labeled_automerge(pr_num: int) -> bool: 'jenkins_unittests_result': get_statuses_result('Brain-Score Jenkins CI', statuses_json)} tests_pass = are_all_tests_passing(results_dict) + tests_fail = any_tests_failing(results_dict) if tests_pass: if is_labeled_automerge(pr_num): @@ -122,4 +147,7 @@ def is_labeled_automerge(pr_num: int) -> bool: else: print("All tests pass but not labeled for automerge. Exiting.") else: + if tests_fail: + if is_labeled_automerge(pr_num): + print(False) print(results_dict)