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

revup explodes on PRs with team reviewer #73

Closed
anguslees opened this issue Oct 12, 2022 · 1 comment · Fixed by #75
Closed

revup explodes on PRs with team reviewer #73

anguslees opened this issue Oct 12, 2022 · 1 comment · Fixed by #75
Assignees
Labels
bug Something isn't working

Comments

@anguslees
Copy link
Contributor

anguslees commented Oct 12, 2022

Describe the bug
revup trips over PRs that have non-User (ie: team) reviewers.

reviewRequests.requestedReviewer can be a user or team. See https://docs.github.com/en/graphql/reference/unions#requestedreviewer

Expected behavior
Support team reviewers, or at least safely ignore non-User reviewers.

This is sufficient to fix it for me:

            for revs in this_node["reviewRequests"]["nodes"]:
+               if revs["requestedReviewer"]:
                    reviewers.add(revs["requestedReviewer"]["login"])
                    reviewer_ids.add(revs["requestedReviewer"]["id"])

To Reproduce
revup upload to create PR. Add a team reviewer to the PR via github UI. Attempt to run revup upload again.

Logs
Terminal output from the command. If possible, rerun the command with revup -v to get more verbose logs.

It's a private repo, so I can't point you at the PR, nor include the full logs. But this PR has a regular reviewer and a team reviewer - and I think the interesting bits of the logs are:

% revup -v upload
[...]
      "reviewRequests": {
       "nodes": [
        {
         "requestedReviewer": {
          "login": "joe_user",
          "id": "M...Q=="
         }
        },
        {
         "requestedReviewer": null
        }
       ]
      },
[...]
Traceback (most recent call last):
  File "/Users/gus/bin/revup", line 8, in <module>
    sys.exit(_main())
  File "/Users/gus/revup/lib/python3.10/site-packages/revup/__main__.py", line 17, in _main
    sys.exit(asyncio.run(main()))
  File "/opt/homebrew/Cellar/python@3.10/3.10.6_2/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/opt/homebrew/Cellar/python@3.10/3.10.6_2/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
    return future.result()
  File "/Users/gus/revup/lib/python3.10/site-packages/revup/revup.py", line 357, in main
    return await upload.main(
  File "/Users/gus/revup/lib/python3.10/site-packages/revup/upload.py", line 46, in main
    await topics.query_github()
  File "/Users/gus/revup/lib/python3.10/site-packages/revup/topic_stack.py", line 961, in query_github
    ) = await github_utils.query_everything(
  File "/Users/gus/revup/lib/python3.10/site-packages/revup/github_utils.py", line 301, in query_everything
    reviewers.add(revs["requestedReviewer"]["login"])
TypeError: 'NoneType' object is not subscriptable
@anguslees anguslees added the bug Something isn't working label Oct 12, 2022
@jerry-skydio jerry-skydio self-assigned this Oct 12, 2022
jerry-skydio added a commit that referenced this issue Oct 12, 2022
jerry-skydio added a commit that referenced this issue Oct 14, 2022
Adding full team support looks like a medium task,
but for now we can at least not crash.

Topic: teams
Reviewers: brian-k
closes #73
jerry-skydio added a commit that referenced this issue Oct 14, 2022
Adding full team support looks like a medium task,
but for now we can at least not crash.

Topic: teams
Reviewers: brian-k
closes #73
jerry-skydio added a commit that referenced this issue Oct 14, 2022
Adding full team support looks like a medium task,
but for now we can at least not crash.

Topic: teams
Reviewers: brian-k
closes #73
@jerry-skydio
Copy link
Collaborator

actually supporting Teams is opened as a new feature #76

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants