-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/bwdo 520 review #3
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
base: develop
Are you sure you want to change the base?
Changes from all commits
28df27e
7d0c838
98c1abd
a7d47a6
f59d29c
10adb4d
274f73c
3923af5
8e1f338
8737e84
b6a9b80
be138b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| # Copyright 2025 RDK Management | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| from dataclasses import dataclass | ||
| from enum import Enum | ||
|
|
||
| class CRStatus(str, Enum): | ||
| OPEN = "Open" | ||
| CLOSED = "Closed" | ||
| MERGED = "Merged" | ||
|
|
||
| def __str__(self): | ||
| return self.value | ||
|
|
||
| @dataclass | ||
| class CodeReview: | ||
| url: str | ||
| status: CRStatus |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| # Copyright 2025 RDK Management | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| class ReviewException(Exception): | ||
| pass | ||
|
|
||
| class TicketNotFound(ReviewException): | ||
| """Raised when a ticket cannot be found. | ||
|
|
||
| Args: | ||
| ticket_url (str): URL of the ticket not found. | ||
| """ | ||
| def __init__(self, ticket_url: str): | ||
| super().__init__(f'Ticket not found at url: {ticket_url}') | ||
|
|
||
| class TicketingInstanceUnreachable(ReviewException): | ||
| """Raised when a ticketing instance is unreachable. | ||
|
|
||
| Args: | ||
| instance_url (str): The URL of the unreachable instance. | ||
| additional_info (str): Info on why the instance was unreachable, defaults to ''. | ||
| """ | ||
| def __init__(self, instance_url: str, additional_info: str = ''): | ||
| super().__init__( | ||
| f'Ticketing instance at {instance_url} is unreachable: {additional_info}' | ||
| ) | ||
|
|
||
| class PermissionsError(ReviewException): | ||
| """Raised when permission is denied. | ||
|
|
||
| Args: | ||
| resource (str): The resource access is denied to. | ||
| resolution_message (str): Additional info about access denial, defaults to ''. | ||
| """ | ||
| def __init__(self, resource: str, resolution_message: str = ''): | ||
| super().__init__( | ||
| f'You do not have permission to access {resource}\n{resolution_message}' | ||
| ) | ||
|
|
||
| class ConfigError(ReviewException): | ||
| """Raised when there is an error with the config. | ||
| """ | ||
| pass | ||
|
|
||
| class TicketIdentifierNotFound(ConfigError): | ||
| """Raised when ticket ID isn't found in the config. | ||
| """ | ||
| pass | ||
|
|
||
| class RemoteUrlNotFound(ConfigError): | ||
| """Raised when remote url matches no patterns in the config. | ||
| """ | ||
| pass | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| from .git_factory import GitFactory | ||
| from .git_instance import GitInstance |
TB-1993 marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,33 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # Copyright 2025 RDK Management | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # limitations under the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| from .instances import GithubInstance, GitlabInstance | ||||||||||||||||||||||||||||||||||||||||||||||||||
| from .git_instance import GitInstance | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| class GitFactory: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _registry = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "github": GithubInstance, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "gitlab": GitlabInstance | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||
| def types(cls) -> list[str]: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return list(cls._registry.keys()) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||||||||||||||
| def create(cls, name: str, token: str, base_url: str | None) -> GitInstance: | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| def create(cls, name: str, token: str, base_url: str | None) -> GitInstance: | |
| def create(cls, name: str, token: str, base_url: str | None) -> GitInstance: | |
| """ | |
| Create a GitInstance for the given provider name. | |
| Parameters | |
| ---------- | |
| name : str | |
| The provider name (e.g. "github", "gitlab"). | |
| token : str | |
| The access token used to authenticate with the provider. | |
| base_url : str | None | |
| Optional base URL for self-hosted or custom deployments. | |
| Notes | |
| ----- | |
| This method is intended to be called with keyword arguments for | |
| clarity, e.g.: | |
| GitFactory.create(name="github", base_url="https://github.com", token="...") | |
| The parameter order in the signature is kept for backwards | |
| compatibility. Callers should not rely on positional arguments. | |
| """ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # Copyright 2025 RDK Management | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
| from ..code_review import CodeReview | ||
|
|
||
| class GitInstance(ABC): | ||
| def __init__(self, token: str, base_url: str | None): | ||
| self._token = token | ||
| self.base_url = base_url.rstrip("/") if base_url else None | ||
|
|
||
| @abstractmethod | ||
| def validate_connection(self) -> bool: | ||
| """Abstract Method: | ||
| Validates if connection to the git instance is successful. | ||
|
|
||
| Raises: | ||
| ConnectionError: If the connection is unsuccessful. | ||
|
|
||
| Returns: | ||
| bool: True if connection is successful. | ||
| """ | ||
| pass | ||
|
|
||
| @abstractmethod | ||
| def get_code_review(self, repo: str, source_branch: str) -> CodeReview: | ||
TB-1993 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| """Get information about a branches code review. | ||
|
|
||
| Args: | ||
| repo (str): An identifier of the repo in the instance e.g "org/repo". | ||
| source_branch (str): The branch the code review is made from. | ||
|
|
||
| Returns: | ||
| CodeReview: dataclass with information about the code review. | ||
| """ | ||
| pass | ||
|
|
||
| @abstractmethod | ||
| def get_create_cr_url( | ||
| self, | ||
| repo: str, | ||
| source_branch: str, | ||
| target_branch: str = "develop" | ||
| ) -> str: | ||
| """Get the URL to create a code review for a repo and branch. | ||
|
|
||
| Args: | ||
| repo (str): An identifier of the repo in the instance e.g "org/repo". | ||
| source_branch (str): The branch the code review will be made from. | ||
| target_branch (str, optional): The branch the code review will | ||
| merge into. Defaults to "develop". | ||
|
|
||
| Returns: | ||
| str: The URL to create a code review. | ||
| """ | ||
| pass | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| from .github_instance import GithubInstance | ||
| from .gitlab_instance import GitlabInstance |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,97 @@ | ||||||||||||||||||||||||||||||||||||||
| # Copyright 2025 RDK Management | ||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||||||
| # you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||||||
| # You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||
| # Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||||||
| # See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||||||
| # limitations under the License. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import requests | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| from sc.review.code_review import CRStatus, CodeReview | ||||||||||||||||||||||||||||||||||||||
| from ..git_instance import GitInstance | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| class GithubInstance(GitInstance): | ||||||||||||||||||||||||||||||||||||||
| def __init__(self, token: str, base_url: str | None): | ||||||||||||||||||||||||||||||||||||||
| super().__init__(token, base_url or "https://api.github.com") | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def _headers(self) -> dict: | ||||||||||||||||||||||||||||||||||||||
TB-1993 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||
| "Accept": "application/vnd.github.v3+json", | ||||||||||||||||||||||||||||||||||||||
| "Authorization": f"Bearer {self._token}" | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def validate_connection(self) -> bool: | ||||||||||||||||||||||||||||||||||||||
| url = f"{self.base_url}/user" | ||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||
| r = requests.get(url, headers=self._headers(), timeout=10) | ||||||||||||||||||||||||||||||||||||||
| r.raise_for_status() | ||||||||||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||||||||||
| except requests.exceptions.Timeout as e: | ||||||||||||||||||||||||||||||||||||||
| raise ConnectionError("GitHub API request timed out.") from e | ||||||||||||||||||||||||||||||||||||||
| except requests.exceptions.HTTPError as e: | ||||||||||||||||||||||||||||||||||||||
| status = e.response.status_code | ||||||||||||||||||||||||||||||||||||||
| if status in (401, 403): | ||||||||||||||||||||||||||||||||||||||
| raise ConnectionError("Invalid GitHub token.") from e | ||||||||||||||||||||||||||||||||||||||
| raise ConnectionError(f"GitHub API error: {status}") from e | ||||||||||||||||||||||||||||||||||||||
| except requests.exceptions.ConnectionError as e: | ||||||||||||||||||||||||||||||||||||||
| raise ConnectionError("Network connection to GitHub failed.") from e | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def get_code_review(self, repo: str, source_branch: str) -> CodeReview | None: | ||||||||||||||||||||||||||||||||||||||
| """Get information about a code review. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||||||
| repo (str): An identifier for the repo e.g. org/repo | ||||||||||||||||||||||||||||||||||||||
| source_branch (str): The source branch of review. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Raises: | ||||||||||||||||||||||||||||||||||||||
| RuntimeError: If an error occurs. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||||
| CodeReview | None: An object describing a code review. | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| url = f"{self.base_url}/repos/{repo}/pulls" | ||||||||||||||||||||||||||||||||||||||
| owner = repo.split("/")[0] | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
| owner = repo.split("/")[0] | |
| repo = repo.strip() | |
| if not repo or "/" not in repo: | |
| raise ValueError("Invalid GitHub repository format. Expected 'owner/name'.") | |
| owner, _ = repo.split("/", 1) |
Copilot
AI
Jan 7, 2026
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.
The repo.split("/")[0] call can raise an IndexError if the repo string doesn't contain a "/" character. Consider adding validation or error handling for malformed repository identifiers.
Copilot
AI
Jan 6, 2026
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.
The variable r is referenced in the error handling blocks (lines 58-59) but may not be defined if the exception occurs during the requests.get() call itself (line 52). This will cause a NameError. Move the status_code and text access into a try-except block or check if r is defined before using it.
| raise RuntimeError( | |
| f"GitHub API error {r.status_code}: {r.text}" | |
| ) from e | |
| response = getattr(e, "response", None) | |
| status = getattr(response, "status_code", "unknown") | |
| text = getattr(response, "text", "") | |
| raise RuntimeError(f"GitHub API error {status}: {text}") from e |
Copilot
AI
Jan 6, 2026
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.
The exception handling catches specific requests.Timeout and requests.HTTPError exceptions, but these don't exist in the requests library. The correct exception classes are requests.exceptions.Timeout and requests.exceptions.HTTPError. This will cause an AttributeError at runtime when these exceptions are raised.
| except requests.Timeout as e: | |
| raise RuntimeError("GitHub request timed out") from e | |
| except requests.HTTPError as e: | |
| raise RuntimeError( | |
| f"GitHub API error {r.status_code}: {r.text}" | |
| ) from e | |
| except ValueError as e: # JSON decode error | |
| raise RuntimeError("Invalid JSON from GitHub API") from e | |
| except requests.RequestException as e: | |
| except requests.exceptions.Timeout as e: | |
| raise RuntimeError("GitHub request timed out") from e | |
| except requests.exceptions.HTTPError as e: | |
| raise RuntimeError( | |
| f"GitHub API error {r.status_code}: {r.text}" | |
| ) from e | |
| except ValueError as e: # JSON decode error | |
| raise RuntimeError("Invalid JSON from GitHub API") from e | |
| except requests.exceptions.RequestException as e: |
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.
The RemoteUrlNotFound exception class is missing a pass statement or docstring body. According to PEP 257, if a class has only a docstring and no methods, it should still have a pass statement for clarity, or the docstring should be on the same line as the class definition if it's a one-liner.