Skip to content

Commit

Permalink
Revert "chore: client error fail without 500 error"
Browse files Browse the repository at this point in the history
This reverts commit 83f589a.
  • Loading branch information
yanksyoon committed Mar 1, 2024
1 parent fe6b392 commit da385e0
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 71 deletions.
22 changes: 5 additions & 17 deletions repo_policy_compliance/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from repo_policy_compliance import log
from repo_policy_compliance.comment import remove_quote_lines
from repo_policy_compliance.github_client import (
GithubClientError,
get_branch,
get_collaborator_permission,
get_collaborators,
Expand Down Expand Up @@ -182,21 +181,14 @@ def _branch_external_fork(repository: Repository, source_repository_name: str) -
Returns:
Whether the branch is from a external fork.
Raises:
GithubClientError: if an unexpected error occurred getting collaborator permission.
"""
if repository.full_name == source_repository_name:
return False

fork_username = source_repository_name.split("/")[0]

# Check if owner of the fork already has push or higher permission (not an external user)
try:
fork_user_permission = get_collaborator_permission(repository, fork_username)
except GithubClientError as exc:
log.logging.error(f"Failed to get collaborator permission: {exc}")
raise
fork_user_permission = get_collaborator_permission(repository, fork_username)
if fork_user_permission in ("admin", "write"):
return False

Expand All @@ -205,7 +197,7 @@ def _branch_external_fork(repository: Repository, source_repository_name: str) -

@inject_github_client
@log.call
def execute_job( # pylint: disable=too-many-return-statements
def execute_job(
github_client: Github,
repository_name: str,
source_repository_name: str,
Expand All @@ -231,13 +223,9 @@ def execute_job( # pylint: disable=too-many-return-statements
repository=repository, permission="push", affiliation="all"
)
}
try:
is_external_fork = _branch_external_fork(
repository=repository, source_repository_name=source_repository_name
)
except GithubClientError as exc:
return Report(result=Result.FAIL, reason=f"Unexpected error from GitHub client: {exc}")
if not is_external_fork:
if not _branch_external_fork(
repository=repository, source_repository_name=source_repository_name
):
return Report(result=Result.PASS, reason=None)

# Retrieve PR for the branch
Expand Down
8 changes: 1 addition & 7 deletions repo_policy_compliance/github_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,7 @@ def get_collaborator_permission(
Returns:
The collaborator permission.
"""
try:
user_permission = repository.get_collaborator_permission(username)
except GithubException as exc:
raise GithubClientError(
"Unexpected error occurred fetching permissions."
f"Message: {exc.message}, status: {exc.status}, data: {exc.data}"
) from exc
user_permission = repository.get_collaborator_permission(username)
if user_permission not in ("admin", "write", "read", "none"):
raise GithubClientError(
f"Invalid collaborator permission {user_permission} received, "
Expand Down
45 changes: 0 additions & 45 deletions tests/integration/test_execute_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,55 +16,10 @@

import repo_policy_compliance
from repo_policy_compliance.check import AUTHORIZATION_STRING_PREFIX, Result, execute_job
from repo_policy_compliance.exceptions import GithubClientError

from .. import assert_


@pytest.mark.parametrize(
"forked_github_branch",
[f"test-branch/execute-job/no-pr/{uuid4()}"],
indirect=True,
)
@pytest.mark.usefixtures("make_fork_branch_external")
def test__branch_external_fork_github_error(
monkeypatch: pytest.MonkeyPatch,
forked_github_repository: Repository,
forked_github_branch: Branch,
github_repository_name: str,
):
"""
arrange: Given a mock _branch_external_fork that raises a Github error.
act: when execute_job is called.
assert: A failed result is returned with Github exception in reason.
"""

def raise_(exception):
"""Raises exception.
Args:
exception: The exception to raise.
"""
raise exception

monkeypatch.setattr(
repo_policy_compliance.check,
"_branch_external_fork",
lambda: raise_(GithubClientError("No user found.")),
)

# The github_client is injected
report = execute_job( # pylint: disable=no-value-for-parameter
repository_name=github_repository_name,
source_repository_name=forked_github_repository.full_name,
branch_name=forked_github_branch.name,
commit_sha=forked_github_branch.commit.sha,
)

assert report.result == Result.FAIL
assert report.reason, "expected a reason along with the fail result"


@pytest.mark.parametrize(
"forked_github_branch",
[f"test-branch/execute-job/no-pr/{uuid4()}"],
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/types_.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ class RequestedCollaborator(NamedTuple):
role_name: The name of the role the collaborators should have
"""

permission: Literal["admin", "pull"] # one of [pull, triage, push, maintain, admin]
permission: Literal["admin", "pull"]
role_name: str
2 changes: 1 addition & 1 deletion tests/unit/test_github_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_get_collaborator_permission_error():

with pytest.raises(GithubClientError) as error:
# The github_client is injected
repo_policy_compliance.github_client.get_collaborator_permission(
repo_policy_compliance.github_client.get_collaborator_permission( # pylint: disable=no-value-for-parameter
mock_repository, "test_user"
)
assert "Invalid collaborator permission" in str(error.value)

0 comments on commit da385e0

Please sign in to comment.