jupyterhub / binderhub

Run your code in the cloud, with technology so advanced, it feels like magic!
https://binderhub.readthedocs.io
BSD 3-Clause "New" or "Revised" License
2.54k stars 388 forks source link

logic for error message for private repo #1658

Closed cranmer closed 1 year ago

cranmer commented 1 year ago

Bug description

If you provide a private repo in GitHub, the error message is not helpful or what is intended. It seems there is a logic error in the code.

Expected behaviour

I'd expect it to say something like Check to make sure this repository is public. The code expects to say "Is your repo public?"

https://github.com/jupyterhub/binderhub/blob/e27f63f8597a84a8cf079de284e6486fabbece88/binderhub/builder.py#L338

Actual behaviour

The actual message is

Could not resolve ref for gh:<username>/<repo>/HEAD. Double check your URL. GitHub recently changed default branches from "master" to "main".

here's the line of code. https://github.com/jupyterhub/binderhub/blob/e27f63f8597a84a8cf079de284e6486fabbece88/binderhub/builder.py#L300

The desired message isn't produced because if provider.name == "GitHub": is True.

How to reproduce

Use any private repository on GitHub in the mybinder interface.

welcome[bot] commented 1 year ago

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively. welcome You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

matthewfeickert commented 1 year ago

The desired message isn't produced because if provider.name == "GitHub": is True.

That bit seems to have been added in PR https://github.com/jupyterhub/binderhub/pull/1170.

An additional problem is that

https://github.com/jupyterhub/binderhub/blob/e27f63f8597a84a8cf079de284e6486fabbece88/binderhub/builder.py#L307

is never triggered if the default of HEAD is used, and so if you have a private repo on GitHub you'll only ever hit

https://github.com/jupyterhub/binderhub/blob/e27f63f8597a84a8cf079de284e6486fabbece88/binderhub/builder.py#L302-L305

and then immediately just return

https://github.com/jupyterhub/binderhub/blob/e27f63f8597a84a8cf079de284e6486fabbece88/binderhub/builder.py#L340-L343

Perhaps an easy fix is just to append

https://github.com/jupyterhub/binderhub/blob/e27f63f8597a84a8cf079de284e6486fabbece88/binderhub/builder.py#L338

directly to

https://github.com/jupyterhub/binderhub/blob/e27f63f8597a84a8cf079de284e6486fabbece88/binderhub/builder.py#L300

given that the user is already being asked to check their URL, and reminder about MyBinder being public use only seems reasonable.

I can PR this (last bit) if people agree.