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

workflows/pr-subscriber: Use our own custom concurrency implementation #66263

Merged
merged 3 commits into from
Sep 14, 2023

Conversation

tstellar
Copy link
Collaborator

The builtin concurrency functionality for the workflows will cancel a pending job if there is another job from the same workflow running. For the pr-subscriber job, this means that if multiple labels are added at the same time, then some of the pr-subscriber jobs will be cancelled and the PR will not have all the necessary mentions.

The builtin concurrency functionality for the workflows will cancel
a pending job if there is another job from the same workflow running.
For the pr-subscriber job, this means that if multiple labels are added
at the same time, then some of the pr-subscriber jobs will be cancelled
and the PR will not have all the necessary mentions.
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 13, 2023

@llvm/pr-subscribers-github-workflow

Changes The builtin concurrency functionality for the workflows will cancel a pending job if there is another job from the same workflow running. For the pr-subscriber job, this means that if multiple labels are added at the same time, then some of the pr-subscriber jobs will be cancelled and the PR will not have all the necessary mentions. -- Full diff: https://github.com//pull/66263.diff

3 Files Affected:

  • (modified) .github/workflows/pr-subscriber.yml (+39-15)
  • (modified) llvm/utils/git/requirements.txt (+9-3)
  • (modified) llvm/utils/git/requirements.txt.in (+1-1)
diff --git a/.github/workflows/pr-subscriber.yml b/.github/workflows/pr-subscriber.yml
index e31b5c448ee1501..c96de150cf036b3 100644
--- a/.github/workflows/pr-subscriber.yml
+++ b/.github/workflows/pr-subscriber.yml
@@ -7,16 +7,9 @@ on:
       - completed
 
 permissions:
+  actions: read
   contents: read
 
-concurrency:
-  # Ideally, we would use the PR number in the concurrency group, but we don't
-  # have access to it here.  We need to ensure only one job is running for
-  # each PR at a time, because there is a potential race condition when
-  # updating the issue comment.
-  group: "PR Subscriber"
-  cancel-in-progress: false
-
 jobs:
   auto-subscribe:
     runs-on: ubuntu-latest
@@ -25,6 +18,44 @@ jobs:
       github.event.workflow_run.event == 'pull_request' &&
       github.event.workflow_run.conclusion == 'success'
     steps:
+      - name: Setup Automation Script
+        run: |
+          curl -O -L https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/github-automation.py
+          curl -O -L https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/requirements.txt
+          chmod a+x github-automation.py
+          pip install -r requirements.txt
+
+      - name: 'Wait for other actions'
+        # We can't use the concurrency tag for these jobs, because it will
+        # cancel pending jobs if another job is running.
+        shell: python
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          import github
+          import os
+          import sys
+          import time
+
+          def needs_to_wait(repo):
+              workflow_name = os.environ.get("GITHUB_WORKFLOW")
+              run_number = os.environ.get("GITHUB_RUN_NUMBER")
+              for status in ["in_progress", "queued"]:
+                  for workflow in repo.get_workflow_runs(status = status):
+                      if workflow.name != workflow_name:
+                          continue
+                      if workflow.run_number < int(run_number):
+                          print("Workflow {} still {} ".format(workflow.run_number, status))
+                          return True
+              return False
+
+          repo_name = os.environ.get("GITHUB_REPOSITORY")
+          token = os.environ.get("GITHUB_TOKEN")
+          gh = github.Github(token)
+          repo = gh.get_repo(repo_name)
+          while needs_to_wait(repo):
+              time.sleep(30)
+
       # From: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
       # Updated version here: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#using-data-from-the-triggering-workflow
       - name: 'Download artifact'
@@ -50,13 +81,6 @@ jobs:
 
       - run: unzip pr.zip
 
-      - name: Setup Automation Script
-        run: |
-          curl -O -L https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/github-automation.py
-          curl -O -L https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/requirements.txt
-          chmod a+x github-automation.py
-          pip install -r requirements.txt
-
       - name: Update watchers
         # https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-an-intermediate-environment-variable
         run: |
diff --git a/llvm/utils/git/requirements.txt b/llvm/utils/git/requirements.txt
index c83b2ae5df2a555..bed449e6bf9f092 100644
--- a/llvm/utils/git/requirements.txt
+++ b/llvm/utils/git/requirements.txt
@@ -9,9 +9,13 @@ certifi==2023.7.22
     #   -r requirements.txt.in
     #   requests
 cffi==1.15.1
-    # via pynacl
+    # via
+    #   cryptography
+    #   pynacl
 charset-normalizer==2.1.1
     # via requests
+cryptography==41.0.3
+    # via pyjwt
 deprecated==1.2.13
     # via pygithub
 gitdb==4.0.9
@@ -22,9 +26,9 @@ idna==3.4
     # via requests
 pycparser==2.21
     # via cffi
-pygithub==1.55
+pygithub==1.59.1
     # via -r requirements.txt.in
-pyjwt==2.5.0
+pyjwt[crypto]==2.5.0
     # via pygithub
 pynacl==1.5.0
     # via pygithub
@@ -32,6 +36,8 @@ requests==2.28.1
     # via pygithub
 smmap==5.0.0
     # via gitdb
+types-cryptography==3.3.23.2
+    # via pyjwt
 urllib3==1.26.12
     # via requests
 wrapt==1.14.1
diff --git a/llvm/utils/git/requirements.txt.in b/llvm/utils/git/requirements.txt.in
index ee45d2349ea1f30..a8bda5c8114058f 100644
--- a/llvm/utils/git/requirements.txt.in
+++ b/llvm/utils/git/requirements.txt.in
@@ -4,5 +4,5 @@
 # pip-compile -o requirements.txt  requirements.txt.in
 
 certifi>=2023.7.22  # https://security.snyk.io/vuln/SNYK-PYTHON-CERTIFI-5805047
-PyGithub
+PyGithub==1.59.1 # For WorkflowRun.name
 GitPython>=3.1.32  # https://security.snyk.io/vuln/SNYK-PYTHON-GITPYTHON-5840584

Copy link
Collaborator

@tru tru left a comment

Choose a reason for hiding this comment

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

Not a super fan of having the python script in the action yaml. Can't we have that checked in instead? I understand we need to fetch it, but it seems more maintainable.

Copy link
Collaborator

@tru tru left a comment

Choose a reason for hiding this comment

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

LGTM

@tstellar tstellar merged commit c532db0 into llvm:main Sep 14, 2023
3 checks passed
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Sep 14, 2023
llvm#66263)

The builtin concurrency functionality for the workflows will cancel a
pending job if there is another job from the same workflow running. For
the pr-subscriber job, this means that if multiple labels are added at
the same time, then some of the pr-subscriber jobs will be cancelled and
the PR will not have all the necessary mentions.
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
llvm#66263)

The builtin concurrency functionality for the workflows will cancel a
pending job if there is another job from the same workflow running. For
the pr-subscriber job, this means that if multiple labels are added at
the same time, then some of the pr-subscriber jobs will be cancelled and
the PR will not have all the necessary mentions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants