You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "sivakumar-mahalingam (via GitHub)" <gi...@apache.org> on 2026/10/18 09:55:28 UTC
[PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
sivakumar-mahalingam opened a new pull request, #4448:
URL: https://github.com/apache/amoro/pull/4448
## Why are the changes needed?
The issue-assignment workflow used the additive `addAssignees` API for every `take` comment. As a result, multiple users could be assigned when they commented `take` on the same issue.
The workflow should assign only the first user who claims an unassigned issue. Subsequent `take` comments must not change its assignees.
Close #4447.
## Brief change log
- Fetch the latest issue state and ignore `take` comments when the issue is already assigned.
- Find the earliest exact `take` comment and assign only its author.
- Prevent concurrent workflow runs from assigning multiple users.
- Await the GitHub assignment API call so failures are reported.
- Update `CONTRIBUTING.md` to document the first-claim behavior.
## How was this patch tested?
- [x] Tested the embedded workflow script with mocked GitHub API scenarios covering the first claim, subsequent claims, an already-assigned issue, and concurrent claims.
- [ ] Add screenshots for manual tests if appropriate (not applicable).
- [x] Ran `actionlint .github/workflows/take.yml`.
- [x] Ran `git diff --check`.
- [x] Ran `./mvnw validate` successfully for all 33 modules.
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
Posted by "sivakumar-mahalingam (via GitHub)" <gi...@apache.org>.
sivakumar-mahalingam commented on PR #4448:
URL: https://github.com/apache/amoro/pull/4448#issuecomment-6336560052
> @sivakumar-mahalingam Thanks for update, The change LGTM.
>
> Currenty, there is still some little probability that the logic will go wrong when there are more then two users comment `take` on the same issue, but this is a rare case and fix it will add some more complexity. so let's go with the current solution.
>
> For the concurrency of workflows we can ref to [control-workflow-concurrency](https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency)
Thanks @klion26 for the review and approval!
Agreed, the remaining concurrency edge case is rare, and the current implementation addresses the original issue without introducing unnecessary complexity.
Thanks also for pointing out GitHub Actions workflow concurrency controls. We can consider further improvements in a separate follow-up if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on code in PR #4448:
URL: https://github.com/apache/amoro/pull/4448#discussion_r4336215273
##########
.github/workflows/take.yml:
##########
@@ -31,7 +31,31 @@ jobs:
- uses: actions/github-script@v7
with:
script: |
- github.rest.issues.addAssignees({
+ const issue = await github.rest.issues.get({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: context.issue.number,
+ })
+
+ if (issue.data.assignees.length > 0) {
+ core.info('The issue is already assigned; ignoring this `take` comment.')
+ return
+ }
+
+ const comments = await github.paginate(github.rest.issues.listComments, {
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: context.issue.number,
+ per_page: 100,
+ })
+ const firstTakeComment = comments.find(comment => comment.body === 'take')
+
+ if (firstTakeComment?.id !== context.payload.comment.id) {
Review Comment:
What will happen for the following case?
1. Alice `take` the issue`
2. Alice was unassigned because some reason(maybe she doesn't have time for now)
3. Now the issue has no assignees
4. What would happen if Bob comment `take` happen?
Seems the `take` in step 4 would not assign the issue to Bob?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on code in PR #4448:
URL: https://github.com/apache/amoro/pull/4448#discussion_r4336215273
##########
.github/workflows/take.yml:
##########
@@ -31,7 +31,31 @@ jobs:
- uses: actions/github-script@v7
with:
script: |
- github.rest.issues.addAssignees({
+ const issue = await github.rest.issues.get({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: context.issue.number,
+ })
+
+ if (issue.data.assignees.length > 0) {
+ core.info('The issue is already assigned; ignoring this `take` comment.')
+ return
+ }
+
+ const comments = await github.paginate(github.rest.issues.listComments, {
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: context.issue.number,
+ per_page: 100,
+ })
+ const firstTakeComment = comments.find(comment => comment.body === 'take')
+
+ if (firstTakeComment?.id !== context.payload.comment.id) {
Review Comment:
What will happen for the following case?
1. Alice `take` the issue`
2. Alice was unassigned because some reason(maybe she doesn't have time for now)
3. Now the issue has no assignees
4. What would happen if Bob comment `take` happen?
Seems the `take` in step 4 would not assign the issue to Bob? We search the previous comment here, as the `take` comment left by Alice is still there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on PR #4448:
URL: https://github.com/apache/amoro/pull/4448#issuecomment-6314839160
@sivakumar-mahalingam Thanks for the reply, I added a inline comment, please take a look at it.
For the question in my last comment, just want to verify the change here is what we want, could you please give an issue link which shows that the change logic here is what we want. thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on PR #4448:
URL: https://github.com/apache/amoro/pull/4448#issuecomment-6265573768
@sivakumar-mahalingam Thanks for the improvement! Does this mean
1. If there is some one took the issue already, the following `take` would not take any side effect
2. If the assignee has been unassigned for some reason(maybe don't have enough time), the following `take` can assign the issue to him/her who comment
3. If there are concurrent `take` comments, the first one win.
And another question, could you please share some pic anywhere, so that we can confirm the behavior? if you need someone else to comment another `take`, please ping me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
Posted by "sivakumar-mahalingam (via GitHub)" <gi...@apache.org>.
sivakumar-mahalingam commented on PR #4448:
URL: https://github.com/apache/amoro/pull/4448#issuecomment-6285643131
Hi @klion26 thanks for reviewing this and for pointing out the unassignment case.
1. If the issue already has an assignee, another `take` comment will **currently** assign it to both users.
2. If the assignee has been unassigned, the following take can assign the issue to him/her who comment 'take'. This behavior is unchanged.
3. For an issue with no previous claim, **currently** concurrent `take` comments are assigned to both users. The pr resolves deterministically and the first comment wins.
This pr raised, as this happened in #4268
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-4447][CI] Prevent multiple assignees from take comments [amoro]
Posted by "sivakumar-mahalingam (via GitHub)" <gi...@apache.org>.
sivakumar-mahalingam commented on PR #4448:
URL: https://github.com/apache/amoro/pull/4448#issuecomment-6248481128
take
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org