Closed tgamblin closed 3 years ago
Huh, this is the error I saw on the newer versions that we were trying to set up spack bot develop for, but now I'm wondering if this is an older error that either was already present, or just appeared. The error seems to be with this call: https://github.com/spack/spackbot/blob/93ef450e59a12c5f42f318e8d1d51b833194e29e/spackbot/handlers/reviewers.py#L266 and that the request is returning not found, which is likely 404. I think it could be that only a specific subset of people are allowed to be added as reviewers (those with write) and it's returning an error if this isn't the case. I'm looking at the endpoint here: https://docs.github.com/en/rest/reference/pulls#request-reviewers-for-a-pull-request and I'm confused why this is a PUT request - as the docs mention GET and POST (and maybe I'm looking at the wrong endpoint, the members_url seems to come from another response). It could also be that we derive a members_url for just one of the two, and the other triggers an error. I didn't write this view - can @adamjstewart comment? It would help to start with feedback from him, and then add some debug statements to look more closely at what is going on (and then fix it).
I'm confused why this is a PUT request - as the docs mention GET and POST (and maybe I'm looking at the wrong endpoint, the members_url seems to come from another response)
This is the endpoint it's constructing: https://docs.github.com/en/rest/reference/teams#add-or-update-team-membership-for-a-user; it's a PUT. It's not adding reviewers -- it's attempting to add non_reviewers
to the maintainers
team.
So, unrelated but FYI:
only a specific subset of people are allowed to be added as reviewers (those with write)
It should be anyone with triage who can be added as a reviewer. See: https://docs.github.com/en/organizations/managing-access-to-your-organizations-repositories/repository-permission-levels-for-an-organization. IIRC the API tells you either "read" or "write", and it doesn't know about triage yet, so we check whether they are repo collaborators... we don't have any teams that "just" add read permissions (see https://github.com/orgs/spack/teams), as read
is redundant for a public repo, so if they're a collaborator they at least have triage
and it adds them.
Anyway, I think I see what the issue is here. jjwilke
has already been invited to the org via that membership add endpoint, and GitHub remembers it (you can see pending invites -- it'll only invite a person once unless we invalidate our invitations).
It doesn't say this in the docs but I think the API is giving us a 404 because they've already been invited -- i.e., you can't PUT a new invite for someone who's already been invited. So the thing to do is probably to just ignore the 404.
Yeah, this is my intuition as well - even regardless of the specific endpoint, it feels like a one off error that happens only under certain circumstances, and we can add a catch with some logging to fix. I'll open a PR this afternoon.
I think this is fixed by #49 -- let's reopen if it's not.
Not sure why, but PRs like https://github.com/spack/spack/pull/25535 result in errors like this when adding maintainers: