Skip to content
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

catch clone_repo exception #225

Merged
merged 2 commits into from
Mar 24, 2022

Conversation

LorisOnori
Copy link
Contributor

@LorisOnori LorisOnori commented Mar 23, 2022

Added GitCommandError exception handler in scan_user. The exception is raised when the repository cannot be cloned. This pull request solves issue #183

@LorisOnori LorisOnori changed the title catch clone_repo exception (#183) catch clone_repo exception Mar 23, 2022
Copy link
Member

@marcorosa marcorosa left a comment

Choose a reason for hiding this comment

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

Hi @LorisOnori I have some requests before merging your pull request.

Moreover, please add a reference to the issue you're solving

Comment on lines 10 to 11
from github import Github
from git import GitCommandError
Copy link
Member

Choose a reason for hiding this comment

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

Please follow alphabetical order of imports (you can use isort if you don't want to do it manually)

Comment on lines 993 to 994
logger.info(f"{i}/{repos_num} Ignore {repo_url} "
"(it can not be cloned)")
Copy link
Member

Choose a reason for hiding this comment

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

please note all the strings are defined by single quotes (') and not double ones (") so we have to be consistent

Copy link
Member

Choose a reason for hiding this comment

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

the logger is in an exception so it shouldn't be at info level.
Better to use either warning or error

@@ -100,6 +100,7 @@ def get_git_repo(self, repo_url, local_repo):
GitRepo.clone_from(repo_url, project_path)
repo = GitRepo(project_path)
except GitCommandError as e:
logger.debug("Repo can not be cloned")
Copy link
Member

Choose a reason for hiding this comment

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

same comments as before, on quotes and logger level

@marcorosa marcorosa merged commit 8b1e966 into develop Mar 24, 2022
@marcorosa marcorosa deleted the fix/scan_user_fails_when_clone_repo_fails branch March 24, 2022 16:34
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.

2 participants