-
Notifications
You must be signed in to change notification settings - Fork 662
[Feature] Dependabot send out issue if package upgrade failure #3512
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
base: master
Are you sure you want to change the base?
Conversation
|
|
||
| jobs: | ||
| monitor-dependabot: | ||
| if: github.actor == 'dependabot[bot]' |
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.
I'm not sure how to trigger Dependabot. During manual testing, this condition was not set.
| if: github.actor == 'dependabot[bot]' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Wait for checks to complete |
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.
This will make sure all of the pipeline is finished before running this script
| console.log("All checks passed."); | ||
| } | ||
| - name: Create issue on failure | ||
| if: failure() && github.event.action == 'opened' |
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.
This is to ensure the bot won't spam the issue.
| const maintainers = collaborators | ||
| .filter(user => user.permissions.admin) | ||
| .map(user => user.login); | ||
|
|
||
| // Step 3: Assign to the PR | ||
| if (maintainers.length > 0) { | ||
| await github.rest.issues.addAssignees({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number, | ||
| assignees: maintainers, | ||
| }); |
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.
This will assign all of the maintainers
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.
curious as of now, who will be assigned?
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.
user.permissions.admin =>["ericl","pcmoritz","richardliaw","Jeffwan","zhe-thoughts"]`
Well, I should assign the reviewer directly, not via the API.
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.
I also try user.permissions.pusher and user.permission.maintainer.
The result is
Pusher: [
'ericl',
'pcmoritz',
'caitengwei',
'wilsonwang371',
'richardliaw',
'Jeffwan',
'zhe-thoughts',
'andrewsykim',
'akanso',
'simon-mo',
'DmitriGekhtman',
'sriram-anyscale'
]
Maintainers: [
'ericl',
'pcmoritz',
'caitengwei',
'richardliaw',
'Jeffwan',
'zhe-thoughts',
'akanso',
'simon-mo',
'DmitriGekhtman'
]
https://github.com/ray-project/kuberay/actions/runs/14836955695/job/41650311396?pr=3544
|
Hi @dentiny, PTAL By the way, if testing is still being considered, Dependabot will be needed. Please let me know, and I’ll find a way to test it properly. |
|
Sorry for the delay, I will take a look later today; feel free to ping me directly on slack if I don't get back to you. Thank you! |
dentiny
left a comment
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.
Could you please link the PR and pipeline in the PR description? Thank you!
| await github.rest.issues.create({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| title: `Dependabot upgrade failed: #${context.issue.number}`, |
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.
nit: include the PR titled in github issue? I think there're pretty well formatted.
| with: | ||
| script: | | ||
| // Step 1: Get collaborators | ||
| const collaborators = await github.paginate( |
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.
Q: do we issue one request or multiple requests, if the overall collaborator numbers are >100?
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, it will automatically issue multiple requests if there are more than 100 collaborators.
| assignees: maintainers, | ||
| }); | ||
| } else { | ||
| console.log("No maintainers found to assign."); |
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.
console.warn
0deb46a to
e4ab607
Compare
|
|
||
| jobs: | ||
| monitor-dependabot: | ||
| if: github.event.pull_request.user.login == 'dependabot[bot]' |
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.
I changed github.actor to github.event.pull_request.user.login
Reference
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const maintainers = ["dentiny"]; |
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.
According to #3512 (comment), we should use fixed maintainer name
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 there are any other users that I should add, please let me know
| await github.rest.issues.create({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| title: `Dependabot upgrade failed: #${context.issue.number} - ${context.payload.pull_request.title}`, |
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.
I have added the pr title
0d4abd3 to
42d8419
Compare
|
Hi @dentiny, I have updated the code
And I have run the pipeline in my repo
PTAL |
Why are these changes needed?
Currently, failed upgrades require manual checking by maintainers. This change aims to improve the process by:
Automatically assigning the repo maintainer if the upgrade succeeds
Automatically creates an issue if the upgrade fails
Manual Testing
Verified the pipeline with a successful case → auto-assigns all maintainers
Verified the pipeline with a failure case → auto-creates an issue
Verified the pipeline with a failure case that includes
reCommit→ does not auto-create an issueAfter the dependabot reopened the issue or re-committed the issue and the pipeline failure, it won't reCreate an issue

Related issue number
Closes #3484
Checks