search

Mic92 / nixpkgs-review

Review pull-requests on https://github.com/NixOS/nixpkgs
MIT License
394 stars 66 forks source link

GitHub token seems to be required now, with bad error message #105

Open lilyball opened 4 years ago

lilyball commented 4 years ago

When I try to run nix-review now I get an HTTP 401 Unauthorized error. I'm not giving nix-review a token, and it should only be accessing public data by default, so I don't know why it's failing. The error is rather obtuse as it's just a very long stack trace but it's definitely coming from the call to the GitHub API.

Mic92 commented 4 years ago

Do you have by chance an out-of-date token in cat $HOME/.config/hub?

lilyball commented 4 years ago

Yes I do. I'm rather surprised that nixpkgs-review would be reading .config/hub.

lilyball commented 4 years ago

Actually, only one of my computers has an outdated token there. The other computer has a perfectly valid token for a GitHub Enterprise installation and no token for github.com. So it looks like nixpkgs-review isn't validating the domain before it grabs the token.

Mic92 commented 4 years ago

Yes. I should add that.

asymmetric commented 4 years ago

Does it need the token at all?

EDIT: I see, it's used for the interaction with GitHub.

lilyball commented 4 years ago

AFAIK it should only be necessary if you instruct nixpkgs-review to leave a comment on the PR. It shouldn't be necessary for the default functionality.

Mic92 commented 4 years ago

Yes it's optional

blaggacao commented 4 years ago

Coming from https://github.com/NixOS/nixpkgs/pull/92612#discussion_r451188524 setting the token on first time usage through prompt would be a good solution.

omasanori commented 4 years ago

It would be great to display which scopes the tool needs, otherwise, one (like me) has to guess the correct combination from dozen of checkboxes. I guess it will be especially painful if that is the first time they issue a new token.

Mic92 commented 4 years ago

@omasanori also discussed here: https://github.com/Mic92/nixpkgs-review/issues/104 You can add it to the readme if you want.

omasanori commented 4 years ago

Thank you for the suggestion, @Mic92! Sure, I will take a look at it.

tko commented 3 months ago

ping.

This still seems to be the behavior out of the box. Tried running literally the first command in README (I'm new here) nix run 'nixpkgs#nixpkgs-review' pr ...

and got 

urllib.error.HTTPError: HTTP Error 401: Unauthorized

at the end of a backtrace.

I happened to have a github token for other purposes and so running GITHUB_TOKEN=... nix run... worked.

Mic92 commented 3 months ago

Maybe, I should just removing hub/gh token parsing.