Skip to content

feat: add import project from remote support #2348

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

Merged
merged 3 commits into from
Nov 1, 2022

Conversation

abhishekmsingh
Copy link
Contributor

resolves #2260

@abhishekmsingh abhishekmsingh marked this pull request as draft October 29, 2022 19:58
@abhishekmsingh abhishekmsingh marked this pull request as ready for review October 31, 2022 20:42
@abhishekmsingh
Copy link
Contributor Author

abhishekmsingh commented Oct 31, 2022

@nejch can you please have a look on this PR?

  • On implementation of the rmote_import function, I have basically copied the logic from import_project, since the common code was not large enough to be refactored, I think into Mixin or a private function. Feel free to suggest any refactoring techinique I can use.
  • On testing, have added kind of a negative functional test which asserts the expected error. I have verified the error from the gitlab spec, here. I am using a ftp url which violates the validate_url predicate there and hence the error.
  • I am not including s3 support here, will add that as separate PR, linked to separate issue(maybe I can raise it), once this goes through. This is my first one here so want this to be a little atomic :)
    Thanks.

@nejch
Copy link
Member

nejch commented Oct 31, 2022

@abhiandthetruth that makes sense! Will check again in a bit, and sure you can go ahead with a separate issue. Yes, currently when we add custom methods it still means copying a bit of code, that needs some rework later.

Copy link
Member

@nejch nejch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again @abhiandthetruth, I had some minor nits but realized we can fix them later with other bits of code.

@abhishekmsingh
Copy link
Contributor Author

Yup will clean them up in the other PR. Thanks! Waiting for my first merge in the repo!

@nejch nejch merged commit e5dc72d into python-gitlab:main Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for remote-import API Endpoint
2 participants