-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Allow import Gitlab repo manually and set a webhook automatically #3934
Conversation
93c0aae
to
573120e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I assume GH/BB do similar already?
Yes. They are working because they always used the Then, because of that decision we found this issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted a spot for some more defensive code.
readthedocs/oauth/services/gitlab.py
Outdated
repo_id = '{username}%2F{repo}'.format( | ||
username=username, | ||
repo=repo, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs error handling and probably some tests. In a case where get_gitlab_username_repo
doesn't match, it returns (None, None)
and we'll search for repo 'None/None'.
When we import a Project manually we don't have a RemoteRepository associated with that Project, so we use the old technique of getting the username and repo names from the URL using `get_gitlab_username_repo` function. This case (Manual Import) won't support custom Gitlab installations in the future with this code.
573120e
to
844b6a9
Compare
I just added a test case for this and rebased the branch. |
Changes and test look good! |
When we import a Project manually we don't have a RemoteRepository associated with that Project, so we use the old technique of getting the username and repo names from the URL using
get_gitlab_username_repo
function.This case (Manual Import) won't support custom Gitlab installations in the future with this code.
Closes https://github.com/readthedocs/readthedocs-corporate/issues/265