From 28df27e8e76543627e9265b3bd969304fc639912 Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Wed, 12 Nov 2025 08:54:37 +0000 Subject: [PATCH 01/12] BWDO-520 Review --- src/sc/cli.py | 3 +- src/sc/review/__init__.py | 0 src/sc/review/exceptions.py | 44 +++ src/sc/review/main.py | 112 ++++++ src/sc/review/review.py | 329 ++++++++++++++++++ src/sc/review/review_config.py | 60 ++++ src/sc/review/ticketing_instances/__init__.py | 3 + .../ticketing_instances/jira_instance.py | 194 +++++++++++ .../ticketing_instances/redmine_instance.py | 192 ++++++++++ src/sc/review/ticketing_instances/ticket.py | 46 +++ .../ticket_instance_factory.py | 27 ++ .../ticketing_instances/ticketing_instance.py | 138 ++++++++ .../review/vcs_instances/github_instance.py | 41 +++ .../review/vcs_instances/gitlab_instance.py | 51 +++ src/sc/review/vcs_instances/pull_request.py | 12 + src/sc/review/vcs_instances/vcs_factory.py | 21 ++ src/sc/review/vcs_instances/vcs_instance.py | 26 ++ src/sc/review_cli.py | 19 + 18 files changed, 1317 insertions(+), 1 deletion(-) create mode 100644 src/sc/review/__init__.py create mode 100644 src/sc/review/exceptions.py create mode 100644 src/sc/review/main.py create mode 100644 src/sc/review/review.py create mode 100644 src/sc/review/review_config.py create mode 100644 src/sc/review/ticketing_instances/__init__.py create mode 100644 src/sc/review/ticketing_instances/jira_instance.py create mode 100644 src/sc/review/ticketing_instances/redmine_instance.py create mode 100644 src/sc/review/ticketing_instances/ticket.py create mode 100644 src/sc/review/ticketing_instances/ticket_instance_factory.py create mode 100644 src/sc/review/ticketing_instances/ticketing_instance.py create mode 100644 src/sc/review/vcs_instances/github_instance.py create mode 100644 src/sc/review/vcs_instances/gitlab_instance.py create mode 100644 src/sc/review/vcs_instances/pull_request.py create mode 100644 src/sc/review/vcs_instances/vcs_factory.py create mode 100644 src/sc/review/vcs_instances/vcs_instance.py create mode 100644 src/sc/review_cli.py diff --git a/src/sc/cli.py b/src/sc/cli.py index 6e595c5..f4028ce 100755 --- a/src/sc/cli.py +++ b/src/sc/cli.py @@ -20,7 +20,7 @@ import click -from . import branching_cli, clone_cli, docker_cli, sc_logging +from . import branching_cli, clone_cli, docker_cli, review_cli, sc_logging CONFIG_DIR = Path(Path.home(), '.sc_config') CONFIG_PATH = Path(CONFIG_DIR, 'config.yaml') @@ -45,6 +45,7 @@ def entry_point(): add_commands_under_cli(branching_cli.cli) add_commands_under_cli(clone_cli.cli) add_commands_under_cli(docker_cli.cli) + add_commands_under_cli(review_cli.cli) cli() diff --git a/src/sc/review/__init__.py b/src/sc/review/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/sc/review/exceptions.py b/src/sc/review/exceptions.py new file mode 100644 index 0000000..cc753cf --- /dev/null +++ b/src/sc/review/exceptions.py @@ -0,0 +1,44 @@ +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 TicketIdentifierNotFound(ReviewException): + """Raised when ticket ID isn't found in the config. + """ + pass + +class RemoteUrlNotFound(ReviewException): + """Raised when remote url matches no patterns in the config. + """ \ No newline at end of file diff --git a/src/sc/review/main.py b/src/sc/review/main.py new file mode 100644 index 0000000..af37b5a --- /dev/null +++ b/src/sc/review/main.py @@ -0,0 +1,112 @@ +import getpass +import logging +from pathlib import Path +import sys + +from git_flow_library import GitFlowLibrary +from repo_library import RepoLibrary +from .exceptions import ReviewException +from .review import Review +from .review_config import ReviewConfig, TicketHostCfg, VcsCfg +from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory +from .vcs_instances.vcs_factory import VcsFactory + +logger = logging.getLogger(__name__) + +def review(): + try: + if root := RepoLibrary.get_repo_root_dir(Path.cwd()): + Review(root.parent).run_repo_command() + elif root := GitFlowLibrary.get_git_root(Path.cwd()): + Review(root.parent).run_git_command() + else: + logger.error("Not in a repo project or git repository!") + sys.exit(1) + except ReviewException as e: + logger.error(e) + sys.exit(1) + +def add_vcs_instance(): + logger.info("Enter VCS provider from the list below: ") + logger.info("github") + logger.info("gitlab") + + provider = input("> ") + print("") + if provider == "github": + url = "http://api.github.com" + elif provider == "gitlab": + logger.info("Enter the URL for the gitlab instance (e.g. https://gitlab.com): ") + url = input("> ") + print("") + else: + logger.error("Provider matches none in the list!") + + logger.info("Enter a pattern for to identify VCS from remote url: ") + logger.info("E.G. github.com for all github instances or github.com/org for a particular organisation") + pattern = input("> ") + print("") + + logger.info("Enter your api token: ") + api_key = input("> ") + print("") + + instance = VcsFactory.create(provider, api_key, url) + + if instance.validate_connection(): + logger.info("Connection validated!") + else: + logger.error("Failed to validate connection!") + sys.exit(1) + + vcs_cfg = VcsCfg(url=url, token=api_key, provider=provider) + ReviewConfig.add_vcs_instance(pattern, vcs_cfg) + + logger.info("VCS Provider Added!") + +def add_ticketing_instance(): + logger.info("Enter the branch prefix (e.g ABC for feature/ABC-123_ticket): ") + branch_prefix = input("> ") + print("") + + logger.info("Enter the ticketing provider from the list below: ") + logger.info("jira") + logger.info("redmine") + provider = input("> ") + print("") + + if provider not in ("jira", "redmine"): + logger.error(f"Provider {provider} not supported!") + sys.exit(1) + + if provider == "jira": + project_prefix = f"{branch_prefix}-" + else: + project_prefix = None + + logger.info("Enter the base URL: ") + base_url = input("> ") + print("") + + logger.info("API token or password: ") + api_token = getpass.getpass("> ") + print("") + + instance = TicketingInstanceFactory.create( + provider=provider, + url=base_url, + token=api_token + ) + if instance.validate_connection(): + logger.info("Connection validated!") + else: + logger.info("Failed to validate connection!") + + ticket_cfg = TicketHostCfg( + url=base_url, + provider=provider, + api_key=api_token, + project_prefix=project_prefix + ) + + ReviewConfig.write_ticketing_data(branch_prefix, ticket_cfg) diff --git a/src/sc/review/review.py b/src/sc/review/review.py new file mode 100644 index 0000000..b344a40 --- /dev/null +++ b/src/sc/review/review.py @@ -0,0 +1,329 @@ +from collections.abc import Iterable +from dataclasses import dataclass +from datetime import datetime +import logging +from pathlib import Path +import re +from urllib import parse + +from git import Repo + +from git_flow_library import GitFlowLibrary + +from sc_manifest_parser import ScManifest +from .exceptions import RemoteUrlNotFound, TicketIdentifierNotFound, TicketNotFound +from .review_config import ReviewConfig +from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory +from .ticketing_instances.ticketing_instance import TicketingInstance +from .vcs_instances.vcs_factory import VcsFactory +from .vcs_instances.vcs_instance import VcsInstance + +logger = logging.getLogger(__name__) + +@dataclass +class CommentData: + branch: str + directory: str | Path + remote_url: str + review_status: str + review_url: str | None + create_pr_url: str + commit_sha: str + commit_author: str + commit_date: datetime + commit_message: str + + def generate_comment(self, formatted: bool = False) -> str: + header = [ + f"Branch: [{self.branch}]", + f"Directory: [{self.directory}]", + f"Git: [{self.remote_url}]", + ] + + review = [ + f"Review Status: [{self.review_status}]", + f"Review URL: [{self.review_url}]" if self.review_url else + f"Create Review URL: [{self.create_pr_url}]" + ] + + commit = ( + f"Last Commit: [{self.commit_sha}]", + f"Author: [{self.commit_author}]", + f"Date: [{self.commit_date}]", + "", + f"{self.commit_message}" + ) + + if formatted: + commit = ["
", *commit, "
"] + + return "\n".join( + [ + *header, + "", + *review, + "", + *commit + ] + ) + +class Review: + def __init__(self, top_dir: str): + self.top_dir = top_dir + + self._config = ReviewConfig() + + def run_git_command(self): + ticket_host_ids = self._config.get_ticket_host_ids() + branch_name = Repo(self.top_dir).active_branch.name + try: + identifier, ticket_num = self._match_branch( + branch_name, + ticket_host_ids + ) + except TicketIdentifierNotFound as e: + logger.warning(e) + identifier, ticket_num = None, None + + if identifier and ticket_num: + ticket_host_data = self._config.get_ticket_host_data(identifier) + ticketing_instance: TicketingInstance = TicketingInstanceFactory.create( + ticket_host_data.class_name, + ticket_host_data.url, + ticket_host_data.api_key, + ticket_host_data.username + ) + + ticket_id = f"{ticket_host_data.project_prefix or ''}{ticket_num}" + ticket = ticketing_instance.read_ticket(ticket_id) + else: + ticketing_instance = None + ticket = None + + vcs_instance = self._create_vcs_instance(Repo(self.top_dir).remote().url) + comment_data = self._create_comment_data(Repo(self.top_dir), vcs_instance) + + logger.info(f"Ticket URL: [{ticket.url if ticket else 'None'}]") + logger.info("Ticket info: ") + print(comment_data.generate_comment()) + + if self._prompt_yn("Update ticket?"): + if ticketing_instance and ticket_id: + ticketing_instance.add_comment_to_ticket( + ticket_id, comment_data.generate_comment(formatted=True)) + else: + ticketing_instance, ticket_id = self._prompt_ticket_selection() + ticketing_instance.add_comment_to_ticket( + ticket_id, comment_data.generate_comment(formatted=True)) + + def run_repo_command(self): + manifest_repo = Repo(self.top_dir / '.repo' / 'manifests') + + ticket_host_ids = self._config.get_ticket_host_ids() + + try: + identifier, ticket_num = self._match_branch( + manifest_repo.active_branch.name, + ticket_host_ids + ) + except TicketIdentifierNotFound as e: + logger.warning(e) + identifier, ticket_num = None, None + + if identifier and ticket_num: + ticket_host_data = self._config.get_ticket_host_data(identifier) + ticketing_instance = TicketingInstanceFactory.create( + ticket_host_data.class_name, + ticket_host_data.url, + ticket_host_data.api_key, + ticket_host_data.username + ) + + ticket_id = f"{ticket_host_data.project_prefix or ''}{ticket_num}" + ticket = ticketing_instance.read_ticket(ticket_id) + else: + ticketing_instance, ticket_id = None, None + + logger.info(f"Ticket URL: [{ticket.url if ticket else ''}]") + logger.info("Ticket info: ") + + manifest = ScManifest.from_repo_root(self.top_dir / '.repo') + comments = [] + for project in manifest.projects: + proj_repo = Repo(self.top_dir / project.path) + proj_vcs = self._create_vcs_instance(proj_repo.remotes[proj_repo.remote].url) + comment_data = self._create_comment_data( + proj_repo, proj_vcs) + comments.append(comment_data) + + manifest_vcs = self._create_vcs_instance(manifest_repo.remote().url) + comment_data = self._create_comment_data( + manifest_repo, manifest_vcs) + comments.append(comment_data) + + multi_repo_comment = self._generate_multi_comment(comments) + print(multi_repo_comment) + + if self._prompt_yn("Update ticket?"): + if ticketing_instance and ticket_id: + ticketing_instance.add_comment_to_ticket( + ticket_id, multi_repo_comment) + else: + ticketing_instance, ticket_id = self._prompt_ticket_selection() + ticketing_instance.add_comment_to_ticket( + ticket_id, multi_repo_comment) + + def _match_branch( + self, + branch_name: str, + host_identifiers: Iterable[str] + ) -> tuple[str, str]: + """Match the branch to an identifier in the config. + + Args: + branch_name (str): The current branch name. + host_identifiers (Iterable[str]): An iterable of ticket host identifiers. + + Raises: + TicketIdentifierNotFound: Raised when the branch doesn't match any + identifiers in the ticket host config. + + Returns: + tuple[str, str]: (matched_identifier, ticket_number) + """ + for identifier in host_identifiers: + # Matches the identifier, followed by - or _, followed by a number + if m := re.search(fr'{identifier}[-_]?(\d+)', branch_name): + ticket_num = m.group(1) + return identifier, ticket_num + raise TicketIdentifierNotFound( + f"Branch {branch_name} doesn't match any ticketing instances! " + f"Found instances {', '.join(host_identifiers)}") + + def _create_vcs_instance(self, remote_url: str) -> VcsInstance: + vcs_url_patterns = self._config.get_vcs_patterns() + try: + remote_pattern = self._match_remote_url( + remote_url, vcs_url_patterns) + except RemoteUrlNotFound as e: + raise RemoteUrlNotFound( + e + f"\nRemotes patterns found: {', '.join(vcs_url_patterns)}" + ) + vcs_data = self._config.get_vcs_data(remote_pattern) + return VcsFactory.create( + vcs_data.provider, + token=vcs_data.token, + base_url=vcs_data.url + ) + + def _match_remote_url( + self, + remote_url: str, + vcs_patterns: Iterable[str] + ) -> str: + """Match the remote url to a pattern in the vcs config. + + Args: + remote_url (str): The remote url of the git repository. + vcs_patterns (Iterable[str]): An iterable of patterns to check against. + + Raises: + RemoteUrlNotFound: Raised when the remote url matches no patterns. + + Returns: + str: The matched pattern. + """ + for pattern in vcs_patterns: + if pattern in remote_url: + return pattern + raise RemoteUrlNotFound(f"{remote_url} doesn't match any patterns!") + + def _get_repo_slug(self, remote_url: str) -> str: + """Return the repository slug (e.g. "org/repo") from a remote url.""" + if remote_url.startswith("git@"): + slug = remote_url.split(":", 1)[1] + else: + slug = parse.urlparse(remote_url).path.lstrip("/") + + if slug.endswith(".git"): + slug = slug[:-4] + + return slug + + def _get_target_branch(self, directory: Path, source_branch: str) -> str: + base = GitFlowLibrary.get_branch_base(source_branch, directory) + return base if base else GitFlowLibrary.get_develop_branch(directory) + + def _prompt_yn(self, msg: str) -> bool: + return input(f"{msg} (y/n): ").strip().lower() == 'y' + + def _create_comment_data(self, repo: Repo, vcs_instance: VcsInstance) -> CommentData: + branch_name = repo.active_branch.name + repo_slug = self._get_repo_slug(repo.remote().url) + pr = vcs_instance.get_pull_request(repo_slug, branch_name) + + target_branch = self._get_target_branch(self.top_dir, branch_name) + create_pr_url = vcs_instance.get_create_pr_url( + repo_slug, branch_name, target_branch) + + commit = repo.head.commit + + review_status = pr.status if pr else "Not Created" + review_url = pr.url if pr else None + + return CommentData( + branch=branch_name, + directory=repo.working_dir, + remote_url=repo.remote().url, + review_status=review_status, + review_url=review_url, + create_pr_url=create_pr_url, + commit_sha=commit.hexsha[:10], + commit_author=f"{commit.author.name} <{commit.author.email}>", + commit_date=commit.committed_datetime, + commit_message=commit.message.strip() + ) + + def _generate_combined_comment( + comments: list[CommentData], formatted: bool = False) -> str: + parts = [] + for c in comments: + parts.append(f"{c.generate_comment(formatted=formatted)}\n") + return "\n".join(parts) + + def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: + """Prompt the user to select a ticketing instance and enter a ticket number. + + Raises: + TicketIdentifierNotFound: If the instance identifier doesn't match any + in the config. + + Returns: + tuple[TicketingInstance, str]: The selected ticketing instance and ticket + ID. + """ + ticket_conf = self._config.get_ticketing_config() + logger.info("Please enter the prefix of the ticket instance:") + logger.info("PREFIX --- INSTANCE URL --- DESCRIPTION") + for id, conf in ticket_conf.items(): + logger.info(f"{id} --- {conf.url} --- {conf.description or ''}") + + input_id = input("> ") + + ticket_instance_conf = ticket_conf.get(input_id) + if not ticket_instance_conf: + raise TicketIdentifierNotFound(f"No prefix matches: {input_id}") + + ticketing_instance = TicketingInstanceFactory.create( + ticket_instance_conf.class_name, + ticket_instance_conf.url, + ticket_instance_conf.api_key, + ticket_instance_conf.username + ) + + logger.info("Please enter your ticket number:") + input_num = input("> ") + + ticket_id = f"{ticket_instance_conf.project_prefix or ''}{input_num}" + + return ticketing_instance, ticket_id diff --git a/src/sc/review/review_config.py b/src/sc/review/review_config.py new file mode 100644 index 0000000..68beab2 --- /dev/null +++ b/src/sc/review/review_config.py @@ -0,0 +1,60 @@ + +from sc.config_manager import ConfigManager + +from pydantic import BaseModel, ConfigDict, ValidationError + +class TicketHostCfg(BaseModel): + model_config = ConfigDict(extra='forbid') + + url: str + class_name: str + username: str | None = None + api_key: str + project_prefix: str | None = None + description: str | None = None + +class VcsCfg(BaseModel): + model_config = ConfigDict(extra='forbid') + + url: str | None = None + token: str + provider: str + +class ReviewConfig: + def __init__(self): + self._ticket_config = ConfigManager('ticketing_instances') + self._vcs_config = ConfigManager('vcs_instances') + + def get_ticketing_config(self) -> dict[str, TicketHostCfg]: + return {k: TicketHostCfg(**v) for k,v in self._ticket_config.get_config().items()} + + def get_ticket_host_ids(self) -> set[str]: + return self._ticket_config.get_config().keys() + + def get_ticket_host_data(self, identifier: str) -> TicketHostCfg: + data = self._ticket_config.get_config().get(identifier) + if not data: + raise KeyError(f"VCS config doesn't contain entry for {identifier}") + try: + return TicketHostCfg(**data) + except ValidationError as e: + raise ValueError(f"Invalid config for {identifier}: {e}") + + def write_ticketing_data(self, branch_prefix: str, ticket_data: TicketHostCfg): + self._ticket_config.update_config({branch_prefix: {ticket_data.model_dump()}}) + + def get_vcs_patterns(self) -> set[str]: + return self._vcs_config.get_config().keys() + + def get_vcs_data(self, url_pattern: str) -> VcsCfg: + data = self._vcs_config.get_config().get(url_pattern) + if not data: + raise KeyError(f"VCS config doesn't contain entry for {url_pattern}") + try: + return VcsCfg(**data) + except ValidationError as e: + raise ValueError(f"Invalid config for {url_pattern}: {e}") + + def write_vcs_data(self, pattern: str, vcs_config: VcsCfg): + self._vcs_config.update_config({pattern: vcs_config.model_dump()}) + \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/__init__.py b/src/sc/review/ticketing_instances/__init__.py new file mode 100644 index 0000000..dd1eee6 --- /dev/null +++ b/src/sc/review/ticketing_instances/__init__.py @@ -0,0 +1,3 @@ +from .jira_instance import JiraInstance +from .redmine_instance import RedmineInstance +from .downloader import Downloader \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py new file mode 100644 index 0000000..8e1dd0e --- /dev/null +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -0,0 +1,194 @@ +#!/usr/bin/env python3 + +from json.decoder import JSONDecodeError +import os +from requests import post +from requests.exceptions import InvalidURL, RequestException +from urllib3 import disable_warnings +from urllib3.exceptions import InsecureRequestWarning + +from jira import JIRA +from jira.exceptions import JIRAError + +from ..exceptions import PermissionsError, TicketingInstanceUnreachable, TicketNotFound +from .ticket import Ticket +from .ticketing_instance import TicketingInstance + + +class JiraInstance(TicketingInstance): + """A class to handle operations on Jira tickets. + + Args: + url (str): URL of Jira instance. + password (str): Password to authenticate with. + + Common KWargs: + verify (bool): Whether to use ssl verification. Default True + extra_setup_options (dict): extra setup options for the jira instance + + Raises: + TicketingInstanceUnreachable: If the ticketing instance cannot be connected to. + """ + + def __init__(self, url: str, password: str, **kwargs): + self._default_jira_setup = { + 'server': '', + 'verify': True, + 'rest_api_version': '2', + } + super().__init__(url, password=password, **kwargs) + self._default_jira_setup['server'] = url + self._default_jira_setup['verify'] = kwargs.get('verify', True) + if isinstance(kwargs.get('extra_setup_options'), dict): + self._default_jira_setup.update(kwargs.get('extra_setup_options')) + # If the user does not want to use SSL verification disable the warnings about it being insecure. + if self._default_jira_setup.get('verify') is False: + disable_warnings(InsecureRequestWarning) + # 9.0.0. is an arbitrary version that hasn't been release yet + # Jira is currently readying version 3 of the api so we should be ready for it + if kwargs.get('version') and self._version_check( + '9.0.0', kwargs.get('version', '0.0.0')) in ('same', 'later'): + self._default_jira_setup['rest_api_version'] = '3' + if kwargs.get('cert') or kwargs.get('certificate'): + self._set_cert(kwargs.get('cert', kwargs.get('certificate'))) + try: + self._instance = JIRA( + options=self._default_jira_setup, + token_auth=self._credentials.get('password'), + proxies=kwargs.get('proxies', None)) + except AttributeError as e: + raise TicketingInstanceUnreachable(url, + additional_info=''.join( + arg for arg in e.args)) + except JIRAError as e: + raise TicketingInstanceUnreachable(e.url, + additional_info=e.status_code) + except JSONDecodeError: + raise TicketingInstanceUnreachable( + url, additional_info='Certificate Response Error') + except InvalidURL as e: + raise TicketingInstanceUnreachable(url, + additional_info=''.join( + arg for arg in e.args)) + + def __del__(self): + if self._default_jira_setup.get('client_cert'): + os.remove(self._default_jira_setup.get('client_cert')) + + @property + def engine(self) -> str: + return 'jira' + + def create_ticket(self, project_key: str, issue_type: str, title: str, + **kwargs): + """Creates a ticket on the Jira instance + + Args: + project_key (str): The projects key on th Jira instance + issue_type (str): The type of issue to raise + titlr (str): Title of the ticket to raise + """ + project_dict = {'project': {'key': project_key}} + kwargs.update(project_dict) + kwargs.update({'issuetype': issue_type, 'summary': title}) + new_issue_id = self._instance.create_issue(fields=kwargs).key + ticket = self.read_ticket(new_issue_id) + return ticket + + def read_ticket(self, ticket_id: str) -> Ticket: + """Reads the contents of the ticket and puts the dictionary in to contents + + Args: + ticket_id (str): The id of the ticket to read. Defaults to None + """ + try: + issue_contents = self._instance.issue(ticket_id).raw['fields'] + except JIRAError as e: + if 'permission' in e.text: + raise PermissionsError(e.url, + 'Please contact the dev-support team') + else: + raise TicketNotFound(e.url) + try: + assignee = issue_contents['assignee'].get('name') + except (KeyError, AttributeError): + assignee = None + author = issue_contents['creator'].get('name') + try: + comments = issue_contents['comment'].get('comments') + except (KeyError, AttributeError): + comments = None + status = issue_contents['status'].get('name') + target_version = ', '.join([version.get('name') for version in issue_contents.get('fixVersions')]) + title = issue_contents.get('summary', '') + ticket_url = '{instance_url}/browse/{ticket_id}'.format( + instance_url=self.url, ticket_id=ticket_id) + ticket = Ticket(ticket_url, + assignee=assignee, + author=author, + comments=comments, + contents=issue_contents, + id=ticket_id, + status=status, + target_version=target_version, + title=title, + url=ticket_url) + return ticket + + def update_ticket(self, ticket_id: str, **kwargs): + """Updates the ticket with the new value for the fields based on the kwargs + + Args: + ticket_id (str): The id of the ticket to update. Defaults to None. + """ + try: + self._instance.issue(ticket_id).update(**kwargs) + except JIRAError as e: + raise TicketNotFound(e.url) + ticket = self.read_ticket(ticket_id) + return ticket + + def add_comment_to_ticket(self, ticket_id: str, comment_message: str): + """Adds a comment to the ticket + + Args: + comment_message (str): The body of the comment + ticket_id (str, optional): The ticket id to add the comment to. Defaults to None. + """ + try: + comment = self._convert_from_html(comment_message) + ticket = self.update_ticket(ticket_id, comment=comment) + except TicketNotFound: + # Some workflows do not accept the above method for adding a comment as it technically rewrites the whole ticket + try: + # Try a post request directly to the add comment REST api instead + content = {'body': comment} + post('{url}/rest/api/2/issue/{ticket_id}/comment'.format( + url=self.url, ticket_id=ticket_id), + json=content, + headers={ + "Authorization": f"Bearer {self._credentials.get('password')}" + }, + cert=self._default_jira_setup.get('client_cert'), + proxies=self._instance._session.proxies) + ticket = self.read_ticket(ticket_id) + except RequestException: + raise TicketNotFound(self.url) + return ticket + + + def _convert_from_html(self, string: str) -> str: + string = string.replace('

', '}') + string = string.replace('

', '{color}') + string = string.replace('
', '{noformat}')
+        string = string.replace('
', '{noformat}') + return string + + def delete_ticket(self, ticket_id: str): + """Revoves the ticket from the Jira instance + + Args: + ticket_id (str, optional): The id of the ticket to delete. Defaults to None. + """ + self._instance.issue(ticket_id).delete() diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/redmine_instance.py new file mode 100644 index 0000000..69ee2cd --- /dev/null +++ b/src/sc/review/ticketing_instances/redmine_instance.py @@ -0,0 +1,192 @@ +#!/usr/bin/env python3 +from urllib3 import disable_warnings +from urllib3.exceptions import InsecureRequestWarning + +from redminelib import Redmine +from redminelib.exceptions import ForbiddenError, ResourceNotFoundError, AuthError +from requests.exceptions import SSLError + +from ..exceptions import TicketNotFound, TicketingInstanceUnreachable, PermissionsError +from .ticketing_instance import TicketingInstance +from .ticket import Ticket + + +class RedmineInstance(TicketingInstance): + """ + A class to handle operations on Redmine tickets. + """ + + def __init__(self, url, password: str, **kwargs): + # Only an api key is necessary for Redmine therefore the username can be empty. + super().__init__(url, password=password, **kwargs) + # If the user does not want to use SSL verification disable the warnings about it being insecure. + if kwargs.get('verify') is False: + disable_warnings(InsecureRequestWarning) + try: + self._instance = Redmine(url, + key=self._credentials.get('password'), + requests={ + 'verify': kwargs.get('verify', True), + 'proxies': kwargs.get('proxies', {}) + }, + version=kwargs.get('version', None)) + except: + raise TicketingInstanceUnreachable(url) + + @property + def engine(self) -> str: + return 'redmine' + + def validate_connection(self) -> bool: + pass + + def create_ticket(self, project_name: str, title: str, **kwargs) -> Ticket: + """Create a ticket on the redmine instance + + Args: + project_name (str): The project to create the ticket under + title (str): The title of the ticket + + Raises: + PermissionsError: User does not have permission to access the ticket on the instances + TicketingInstanceUnreachable: The redmine instance is unreachable + """ + try: + new_ticket = self._instance.issue.create(project_id=project_name, + subject=title, + **kwargs) + except (ResourceNotFoundError, SSLError) as exception: + raise TicketingInstanceUnreachable( + self.url, + additional_information=''.join( + arg for arg in exception.args)) from exception + except ForbiddenError as exception: + raise PermissionsError( + '{url}/{project}'.format(url=self.url, project=project_name), + 'Please contact the dev-support team') + ticket_id = new_ticket + ticket = self.read_ticket(ticket_id) + return ticket + + def read_ticket(self, ticket_id: str) -> Ticket: + """Read the information from a ticket and put it's contents in this object contents dict + + Args: + ticket_id (str): The ticket number to read. + Raises: + TicketNotFound: Ticket not found on the redmine instance + PermissionsError: User does not have permission to access the ticket on the instances + TicketingInstanceUnreachable: The redmine instance is unreachable + """ + ticket_url = self.url + '/issues/' + ticket_id + try: + issue = self._instance.issue.get(ticket_id, include=['journals']) + except ResourceNotFoundError as exception: + raise TicketNotFound(ticket_url) from exception + except (AuthError, ForbiddenError) as exception: + raise PermissionsError( + ticket_url, + 'Please contact the dev-support team') from exception + except SSLError as exception: + raise TicketingInstanceUnreachable( + ticket_url, + additional_info=''.join(str(arg) for arg in exception.args)) + issue_contents = dict((k, v) for k, v in list(issue)) + author = issue_contents['author'].get('name') + try: + assignee = issue_contents['assigned_to'].get('name') + except KeyError: + assignee = None + comments = issue_contents.get('journals') + status = issue_contents['status'].get('name') + try: + target_version = issue_contents['fixed_version'].get('name') + except KeyError: + target_version = None + title = issue_contents.get('subject') + ticket = Ticket(ticket_url, + author=author, + assignee=assignee, + comments=comments, + contents=issue_contents, + id=ticket_id, + status=status, + title=title, + url=ticket_url, + target_version=target_version) + return ticket + + def update_ticket(self, ticket_id, **kwargs): + """Writes the changed fields from the kwargs, back to the ticket + + Raises: + TicketNotFound: Ticket not found on the redmine instance + PermissionsError: User does not have permission to access the ticket on the instances + TicketingInstanceUnreachable: The redmine instance is unreachable + """ + issue_url = '{url}/issues/{ticket_number}'.format( + url=self.url, ticket_number=ticket_id) + try: + self._instance.issue.update(ticket_id, **kwargs) + except ResourceNotFoundError as exception: + raise TicketNotFound(issue_url) from exception + except ForbiddenError as exception: + raise PermissionsError( + issue_url, 'Please contact the dev-support team') from exception + except SSLError as exception: + raise TicketingInstanceUnreachable( + issue_url, + additional_info=''.join(str(arg) for arg in exception.args)) + ticket = self.read_ticket(ticket_id) + return ticket + + def add_comment_to_ticket(self, ticket_id: str, comment_message: str) -> None: + """Add a comment to a ticket on the redmine instance + + Args: + comment_message (str): The message to add as a comment + ticket_id (str): The ticket number to add the comment to. + """ + ticket = self.update_ticket(ticket_id,notes=self._convert_html_colours(comment_message)) + return ticket + + def _convert_html_colours(self, string: str) -> str: + """Convert a html colour tags to redmine formatted colour tags. + + Args: + string (str): html formatted string. + + Returns: + str: Input string with html colour tags converted to redmine colour tags. + """ + string = string.replace('

', '}') + string = string.replace('

', r'%') + return string + + def delete_ticket(self, ticket_id: str): + """Delete a ticket from the redmine instance + + Args: + ticket_id (str, optional): The ticket number to delete. Defaults to None. + + Raises: + TicketNotFound: Ticket not found on the redmine instance + PermissionsError: User does not have permission to access the ticket on the instances + TicketingInstanceUnreachable: The redmine instance is unreachable + """ + issue_url = '{url}/issues/{ticket_number}'.format( + url=self._instance.url, ticket_number=self.ticket_id) + try: + self._instance.issue.delete(self.ticket_id) + except ResourceNotFoundError as exception: + raise TicketNotFound(issue_url) from exception + except ForbiddenError as exception: + raise PermissionsError( + '{url}/issues/{ticket_number}'.format(url=self._url), + 'Please contact the dev-support team') from exception + except SSLError as exception: + raise TicketingInstanceUnreachable( + issue_url, + additional_info=''.join(str(arg) for arg in exception.args)) + return True diff --git a/src/sc/review/ticketing_instances/ticket.py b/src/sc/review/ticketing_instances/ticket.py new file mode 100644 index 0000000..89de025 --- /dev/null +++ b/src/sc/review/ticketing_instances/ticket.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 + +class Ticket(): + + def __init__(self, ticket_url: str, **kwargs): + self._url = ticket_url + self._contents = kwargs + + @property + def author(self): + return self._contents.get('author') + + @property + def assignee(self): + return self._contents.get('assignee') + + @property + def comments(self): + return self._contents.get('comments') + + @property + def contents(self): + return self._contents + + @property + def id(self): + return self._contents.get('id') + + @property + def status(self): + return self._contents.get('status') + + @property + def target_version(self): + return self._contents.get('target_version') + + @property + def title(self) -> str: + return self._contents.get('title') + + @property + def url(self) -> str: + return self._contents.get('url') + + def get(self, undeclared_property): + return self._contents['contents'].get(undeclared_property) diff --git a/src/sc/review/ticketing_instances/ticket_instance_factory.py b/src/sc/review/ticketing_instances/ticket_instance_factory.py new file mode 100644 index 0000000..98ef761 --- /dev/null +++ b/src/sc/review/ticketing_instances/ticket_instance_factory.py @@ -0,0 +1,27 @@ +from sc.review.exceptions import TicketIdentifierNotFound +from .jira_instance import JiraInstance +from .redmine_instance import RedmineInstance +from .ticketing_instance import TicketingInstance + +class TicketingInstanceFactory: + _registry = { + "redmine": RedmineInstance, + "jira": JiraInstance + } + + @classmethod + def providers(cls) -> list[str]: + return list(cls._registry.keys()) + + @classmethod + def create( + cls, + provider: str, + url: str, + token: str, + ) -> TicketingInstance: + try: + return cls._registry[provider](url=url, password=token) + except KeyError: + raise TicketIdentifierNotFound( + f"Provider {provider} doesn't match any ticketing instance!") diff --git a/src/sc/review/ticketing_instances/ticketing_instance.py b/src/sc/review/ticketing_instances/ticketing_instance.py new file mode 100644 index 0000000..b42887e --- /dev/null +++ b/src/sc/review/ticketing_instances/ticketing_instance.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +from abc import ABC, abstractmethod +from os.path import abspath, exists + +from .ticket import Ticket + + +class TicketingInstance(ABC): + """ + A class to handle the ticket(s) that the source control commands are + operating on. + """ + + @abstractmethod + def __init__(self, url: str, **kwargs): + self._credentials = {} + self._url = url + if 'username' and 'password' in kwargs.keys(): + self.set_credentials(kwargs.get('username'), kwargs.get('password')) + + @property + def url(self): + return self._url + + @property + @abstractmethod + def engine(self) -> str: + pass + + @abstractmethod + def create_ticket(self) -> Ticket: + """Abstract Method: + Create a ticket on the ticketing instance. + Read it's contents return the new ticket object + + Returns: + ticket (Ticket): The newly created ticket object + """ + # create ticket + # read tickets contents + # return ticket object + pass + + @abstractmethod + def read_ticket(self, ticket_id: str) -> Ticket: + """Abstract Method: + Read the ticket and return it as a ticket object + + Args: + ticket_id (str): The id of the ticket to update. + Returns: + ticket (Ticket): ticket object. + """ + pass + + @abstractmethod + def update_ticket(self, ticket_id, **kwargs) -> Ticket: + """Abstract Method: + Updates the contents dictionary. + Should update the tickets with the changes in the kwargs. + Reads the updated ticket and returns it as a new ticket object + + Args: + ticket_id (str): The id of the ticket to update. + Returns: + ticket (Ticket): New ticket object with update applied. + """ + pass + + @abstractmethod + def add_comment_to_ticket(self, ticket_id: str, + comment_message: str) -> Ticket: + """Abstract Method: + Adds a comment to the ticket on the ticketing instance. + Reads the new ticket with the new comment. + Returns new ticket object with comment added + + Args: + ticket_id (str): The id of the ticket to update. + comment_message (str): The comment to add to the ticket. + + Returns: + ticket (Ticket): New ticket object with comment added + """ + pass + + @abstractmethod + def delete_ticket(self, ticket_id: str) -> bool: + """Abstract Method: + Should remove the ticket from the instance + + Args: + ticket_id (str, optional): The id of the ticket to remove. + + Returns: + result (bool): True if ticket removed successfully. + """ + pass + + def set_credentials(self, username: str, password: str): + """Set the username and password for in the credentials dictionary. + + Args: + username (str): Username of the user to login to the ticketing instance with + password (str): Password/API key of the user to login to the ticketing instance with + """ + self._credentials = {'username': username, 'password': password} + + def _version_check(self, required_version: str, + instance_version: str) -> str: + """Compares the versions. + + Args: + required_version (str): Version to compare against + instance_version (str): Version to check + + Returns: + str: (later, same, ealier) depending on the comparison of the versions + """ + if required_version.count('.') < 2: + required_version += '.0.0' + if instance_version.count('.') < 2: + instance_version += '.0.0' + required_major, required_minor, required_bug = required_version.split( + '.') + instance_major, instance_minor, instance_bug = instance_version.split( + '.') + if instance_version == required_version: + return 'same' + if int(instance_major) > int(required_major): + return 'later' + elif int(instance_major) == int(required_major): + if int(instance_minor) > int(required_minor): + return 'later' + elif int(instance_minor) == int(required_minor): + if int(instance_bug) > int(required_bug): + return 'later' + return 'earlier' diff --git a/src/sc/review/vcs_instances/github_instance.py b/src/sc/review/vcs_instances/github_instance.py new file mode 100644 index 0000000..66ea8a2 --- /dev/null +++ b/src/sc/review/vcs_instances/github_instance.py @@ -0,0 +1,41 @@ +import requests + +from .pull_request import PRStatus, PullRequest +from .vcs_instance import VcsInstance + +class GithubInstance(VcsInstance): + def __init__(self, token: str, base_url: str | None): + super().__init__(token, base_url or "https://api.github.com") + + def _headers(self) -> dict: + return { + "Accept": "application/vnd.github.v3+json", + "Authorization": f"Bearer {self.token}" + } + + def get_pull_request(self, repo: str, source_branch: str) -> PullRequest | None: + url = f"{self.base_url}/repos/{repo}/pulls" + params = {"state": "all", "head": f"{repo}:{source_branch}"} + r = requests.get(url, headers=self._headers(), params=params, timeout=10) + r.raise_for_status() + prs = r.json() + if not prs: + return None + pr = prs[0] + # GitHub marks merged PRs as state="closed", merged=True + if pr.get("merged"): + status = PRStatus.MERGED + elif pr["state"] == "open": + status = PRStatus.OPEN + else: + status = PRStatus.CLOSED + + return PullRequest(url=pr["html_url"], status=status) + + def get_create_pr_url( + self, + repo: str, + source_branch: str, + target_branch: str = "develop" + ) -> str: + return f"https://github.com/{repo}/compare/{target_branch}...{source_branch}" \ No newline at end of file diff --git a/src/sc/review/vcs_instances/gitlab_instance.py b/src/sc/review/vcs_instances/gitlab_instance.py new file mode 100644 index 0000000..a6283de --- /dev/null +++ b/src/sc/review/vcs_instances/gitlab_instance.py @@ -0,0 +1,51 @@ +import requests +import urllib.parse + +from .pull_request import PullRequest, PRStatus +from .vcs_instance import VcsInstance + +class GitlabInstance(VcsInstance): + def __init__(self, token: str, base_url: str): + super().__init__(token, base_url) + + def _headers(self) -> dict[str, str]: + return {"Private-Token": self.token} + + def validate_connection(self) -> bool: + try: + url = f"{self.base_url}/api/v4/user" + r = requests.get(url, headers=self._headers(), timeout=5) + r.raise_for_status() + return True + except requests.RequestException: + return False + + def get_pull_request(self, repo: str, source_branch: str) -> PullRequest: + safe_repo = urllib.parse.quote(repo, safe='') + url = f"{self.base_url}/api/v4/projects/{safe_repo}/merge_requests" + params = {"state": "all", "source_branch": source_branch} + r = requests.get(url, headers=self._headers(), params=params, timeout=10) + r.raise_for_status() + prs = r.json() + if not prs: + return None + pr = prs[0] + + state = pr["state"] + if state == "merged": + status = PRStatus.MERGED + elif state == "opened": + status = PRStatus.OPEN + else: + status = PRStatus.CLOSED + + return PullRequest(url=pr["web_url"], status=status) + + def get_create_pr_url(self, repo, source_branch, target_branch = "develop"): + safe_repo = urllib.parse.quote(repo, safe="") + params = { + "merge_request[source_branch]": source_branch, + "merge_request[target_branch]": target_branch, + } + query = urllib.parse.urlencode(params) + return f"{self.base_url}/{safe_repo}/-/merge_requests/new?{query}" \ No newline at end of file diff --git a/src/sc/review/vcs_instances/pull_request.py b/src/sc/review/vcs_instances/pull_request.py new file mode 100644 index 0000000..8f3c79d --- /dev/null +++ b/src/sc/review/vcs_instances/pull_request.py @@ -0,0 +1,12 @@ +from dataclasses import dataclass +from enum import Enum + +class PRStatus(Enum): + OPEN = "open" + CLOSED = "closed" + MERGED = "merged" + +@dataclass +class PullRequest: + url: str + status: PRStatus \ No newline at end of file diff --git a/src/sc/review/vcs_instances/vcs_factory.py b/src/sc/review/vcs_instances/vcs_factory.py new file mode 100644 index 0000000..206751b --- /dev/null +++ b/src/sc/review/vcs_instances/vcs_factory.py @@ -0,0 +1,21 @@ + +from .github_instance import GithubInstance +from .gitlab_instance import GitlabInstance +from .vcs_instance import VcsInstance + +class VcsFactory: + _registry = { + "github": GithubInstance, + "gitlab": GitlabInstance + } + + @classmethod + def names(cls) -> list[str]: + return list(cls._registry.keys()) + + @classmethod + def create(cls, name: str, token: str, base_url: str | None) -> VcsInstance: + try: + return cls._registry[name.lower()](token=token, base_url=base_url) + except KeyError: + raise ValueError(f"Provider name {name} doesn't match any VCS instance!") \ No newline at end of file diff --git a/src/sc/review/vcs_instances/vcs_instance.py b/src/sc/review/vcs_instances/vcs_instance.py new file mode 100644 index 0000000..6482695 --- /dev/null +++ b/src/sc/review/vcs_instances/vcs_instance.py @@ -0,0 +1,26 @@ +from abc import ABC, abstractmethod + +from .pull_request import PullRequest + +class VcsInstance(ABC): + @abstractmethod + 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: + pass + + @abstractmethod + def get_pull_request(self, repo: str, source_branch: str) -> PullRequest: + pass + + @abstractmethod + def get_create_pr_url( + self, + repo: str, + source_branch: str, + target_branch: str = "develop" + ): + pass \ No newline at end of file diff --git a/src/sc/review_cli.py b/src/sc/review_cli.py new file mode 100644 index 0000000..b4f558c --- /dev/null +++ b/src/sc/review_cli.py @@ -0,0 +1,19 @@ +import click + +from .review import main + +@click.group() +def cli(): + pass + +@cli.command() +def review(): + main.review() + +@cli.command() +def add_vcs_instance(): + main.add_vcs_instance() + +@cli.command() +def add_ticketing_instance(): + main.add_ticketing_instance() \ No newline at end of file From 7d0c838e21f6100fe2681068e2437917081ab56a Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Tue, 18 Nov 2025 08:52:57 +0000 Subject: [PATCH 02/12] BWDO-520 - sc review --- setup.py | 8 +- .../git_factory.py} | 6 +- .../git_instance.py} | 11 +- .../github_instance.py | 24 +- .../gitlab_instance.py | 20 +- src/sc/review/git_instances/pull_request.py | 15 ++ src/sc/review/main.py | 66 +++-- src/sc/review/review.py | 251 ++++++++++-------- src/sc/review/review_config.py | 30 +-- src/sc/review/ticketing_instances/__init__.py | 3 - .../ticketing_instances/jira_instance.py | 25 ++ .../ticketing_instances/redmine_instance.py | 20 +- .../ticket_instance_factory.py | 3 +- .../ticketing_instances/ticketing_instance.py | 23 ++ src/sc/review/vcs_instances/pull_request.py | 12 - src/sc/review_cli.py | 7 +- 16 files changed, 331 insertions(+), 193 deletions(-) rename src/sc/review/{vcs_instances/vcs_factory.py => git_instances/git_factory.py} (88%) rename src/sc/review/{vcs_instances/vcs_instance.py => git_instances/git_instance.py} (67%) rename src/sc/review/{vcs_instances => git_instances}/github_instance.py (58%) rename src/sc/review/{vcs_instances => git_instances}/gitlab_instance.py (70%) create mode 100644 src/sc/review/git_instances/pull_request.py delete mode 100644 src/sc/review/vcs_instances/pull_request.py diff --git a/setup.py b/setup.py index 7bdaf92..7d0526d 100644 --- a/setup.py +++ b/setup.py @@ -30,12 +30,14 @@ def read_version(): package_dir={"": "src"}, install_requires=[ 'Click>=8', - 'pyyaml~=6.0', - 'rich>=14', - 'requests==2.31.0', # Docker SDK breaks on 2.32.0 'docker~=7.0', 'gitpython>=3', + 'jira>=3.10', 'pydantic>=2', + 'python-redmine>=2.5', + 'pyyaml~=6.0', + 'rich>=14', + 'requests==2.31.0', # Docker SDK breaks on 2.32.0 'repo_library @ git+https://github.com/rdkcentral/sc-repo-library.git@master', 'git_flow_library @ git+https://github.com/rdkcentral/sc-git-flow-library.git@master', 'sc_manifest_parser @ git+https://github.com/rdkcentral/sc-manifest-parser.git@main' diff --git a/src/sc/review/vcs_instances/vcs_factory.py b/src/sc/review/git_instances/git_factory.py similarity index 88% rename from src/sc/review/vcs_instances/vcs_factory.py rename to src/sc/review/git_instances/git_factory.py index 206751b..7555f5b 100644 --- a/src/sc/review/vcs_instances/vcs_factory.py +++ b/src/sc/review/git_instances/git_factory.py @@ -1,9 +1,9 @@ from .github_instance import GithubInstance from .gitlab_instance import GitlabInstance -from .vcs_instance import VcsInstance +from .git_instance import GitInstance -class VcsFactory: +class GitFactory: _registry = { "github": GithubInstance, "gitlab": GitlabInstance @@ -14,7 +14,7 @@ def names(cls) -> list[str]: return list(cls._registry.keys()) @classmethod - def create(cls, name: str, token: str, base_url: str | None) -> VcsInstance: + def create(cls, name: str, token: str, base_url: str | None) -> GitInstance: try: return cls._registry[name.lower()](token=token, base_url=base_url) except KeyError: diff --git a/src/sc/review/vcs_instances/vcs_instance.py b/src/sc/review/git_instances/git_instance.py similarity index 67% rename from src/sc/review/vcs_instances/vcs_instance.py rename to src/sc/review/git_instances/git_instance.py index 6482695..44420d2 100644 --- a/src/sc/review/vcs_instances/vcs_instance.py +++ b/src/sc/review/git_instances/git_instance.py @@ -2,7 +2,7 @@ from .pull_request import PullRequest -class VcsInstance(ABC): +class GitInstance(ABC): @abstractmethod def __init__(self, token: str, base_url: str | None): self.token = token @@ -10,6 +10,15 @@ def __init__(self, token: str, base_url: str | None): @abstractmethod def validate_connection(self) -> bool: + """Abstract Method: + Validates if connection to the git instance is successful. + + Raises: + ConnectionError: If the connection is unsuccesful. + + Returns: + bool: True if connection is successful. + """ pass @abstractmethod diff --git a/src/sc/review/vcs_instances/github_instance.py b/src/sc/review/git_instances/github_instance.py similarity index 58% rename from src/sc/review/vcs_instances/github_instance.py rename to src/sc/review/git_instances/github_instance.py index 66ea8a2..abb95c1 100644 --- a/src/sc/review/vcs_instances/github_instance.py +++ b/src/sc/review/git_instances/github_instance.py @@ -1,9 +1,9 @@ import requests from .pull_request import PRStatus, PullRequest -from .vcs_instance import VcsInstance +from .git_instance import GitInstance -class GithubInstance(VcsInstance): +class GithubInstance(GitInstance): def __init__(self, token: str, base_url: str | None): super().__init__(token, base_url or "https://api.github.com") @@ -13,12 +13,28 @@ def _headers(self) -> dict: "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) + if r.status_code == 401: + raise ConnectionError("Invalid GitHub token.") + elif r.status_code >= 400: + raise ConnectionError(f"GitHub API error: {r.status_code}") + return True + except requests.exceptions.Timeout as e: + raise ConnectionError("GitHub API request timed out.") from e + except requests.exceptions.ConnectionError as e: + raise ConnectionError("Network connection to GitHub failed.") from e + def get_pull_request(self, repo: str, source_branch: str) -> PullRequest | None: url = f"{self.base_url}/repos/{repo}/pulls" - params = {"state": "all", "head": f"{repo}:{source_branch}"} + owner = repo.split("/")[0] + params = {"state": "all", "head": f"{owner}:{source_branch}"} r = requests.get(url, headers=self._headers(), params=params, timeout=10) r.raise_for_status() prs = r.json() + print(r.json()) if not prs: return None pr = prs[0] @@ -29,7 +45,7 @@ def get_pull_request(self, repo: str, source_branch: str) -> PullRequest | None: status = PRStatus.OPEN else: status = PRStatus.CLOSED - + return PullRequest(url=pr["html_url"], status=status) def get_create_pr_url( diff --git a/src/sc/review/vcs_instances/gitlab_instance.py b/src/sc/review/git_instances/gitlab_instance.py similarity index 70% rename from src/sc/review/vcs_instances/gitlab_instance.py rename to src/sc/review/git_instances/gitlab_instance.py index a6283de..d29af2c 100644 --- a/src/sc/review/vcs_instances/gitlab_instance.py +++ b/src/sc/review/git_instances/gitlab_instance.py @@ -2,9 +2,9 @@ import urllib.parse from .pull_request import PullRequest, PRStatus -from .vcs_instance import VcsInstance +from .git_instance import GitInstance -class GitlabInstance(VcsInstance): +class GitlabInstance(GitInstance): def __init__(self, token: str, base_url: str): super().__init__(token, base_url) @@ -12,13 +12,19 @@ def _headers(self) -> dict[str, str]: return {"Private-Token": self.token} def validate_connection(self) -> bool: + url = f"{self.base_url}/api/v4/user" try: - url = f"{self.base_url}/api/v4/user" - r = requests.get(url, headers=self._headers(), timeout=5) - r.raise_for_status() + r = requests.get(url, headers=self._headers(), timeout=10) + if r.status_code == 401 or r.status_code == 403: + raise ConnectionError("Invalid GitLab token or insufficient permissions.") + elif r.status_code >= 400: + raise ConnectionError(f"GitLab API error: {r.status_code}") return True - except requests.RequestException: - return False + except requests.exceptions.Timeout as e: + raise ConnectionError("GitLab API request timed out.") from e + except requests.exceptions.ConnectionError as e: + raise ConnectionError("Network connection to GitLab failed.") from e + def get_pull_request(self, repo: str, source_branch: str) -> PullRequest: safe_repo = urllib.parse.quote(repo, safe='') diff --git a/src/sc/review/git_instances/pull_request.py b/src/sc/review/git_instances/pull_request.py new file mode 100644 index 0000000..d5691ca --- /dev/null +++ b/src/sc/review/git_instances/pull_request.py @@ -0,0 +1,15 @@ +from dataclasses import dataclass +from enum import Enum + +class PRStatus(str, Enum): + OPEN = "Open" + CLOSED = "Closed" + MERGED = "Merged" + + def __str__(self): + return self.value + +@dataclass +class PullRequest: + url: str + status: PRStatus \ No newline at end of file diff --git a/src/sc/review/main.py b/src/sc/review/main.py index af37b5a..9389c4f 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -7,9 +7,9 @@ from repo_library import RepoLibrary from .exceptions import ReviewException from .review import Review -from .review_config import ReviewConfig, TicketHostCfg, VcsCfg +from .review_config import ReviewConfig, TicketHostCfg, GitInstanceCfg from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory -from .vcs_instances.vcs_factory import VcsFactory +from .git_instances.git_factory import GitFactory logger = logging.getLogger(__name__) @@ -26,49 +26,53 @@ def review(): logger.error(e) sys.exit(1) -def add_vcs_instance(): - logger.info("Enter VCS provider from the list below: ") +def add_git_instance(): + logger.info("Enter Git provider from the list below: ") logger.info("github") logger.info("gitlab") provider = input("> ") print("") + if provider == "github": url = "http://api.github.com" + logger.info("Enter a pattern for to identify Git from remote url: ") + logger.info( + "E.G. github.com for all github instances or " + "github.com/org for a particular organisation") + pattern = input("> ") + print("") elif provider == "gitlab": - logger.info("Enter the URL for the gitlab instance (e.g. https://gitlab.com): ") + logger.info( + "Enter the URL for the gitlab instance (e.g. https://gitlab.com " + "or https://your-instance.com): ") url = input("> ") print("") + pattern = url.replace("https://", "").replace("http://", "") else: logger.error("Provider matches none in the list!") - logger.info("Enter a pattern for to identify VCS from remote url: ") - logger.info("E.G. github.com for all github instances or github.com/org for a particular organisation") - pattern = input("> ") - print("") logger.info("Enter your api token: ") api_key = input("> ") print("") - instance = VcsFactory.create(provider, api_key, url) + instance = GitFactory.create(provider, api_key, url) - if instance.validate_connection(): - logger.info("Connection validated!") - else: - logger.error("Failed to validate connection!") + try: + instance.validate_connection() + except ConnectionError as e: + logger.error(f"Failed to connect! {e}") sys.exit(1) - vcs_cfg = VcsCfg(url=url, token=api_key, provider=provider) - ReviewConfig.add_vcs_instance(pattern, vcs_cfg) + logger.info("Connection validated!") + + git_cfg = GitInstanceCfg(url=url, token=api_key, provider=provider) + ReviewConfig().write_git_data(pattern, git_cfg) - logger.info("VCS Provider Added!") + logger.info("Git Provider Added!") def add_ticketing_instance(): - logger.info("Enter the branch prefix (e.g ABC for feature/ABC-123_ticket): ") - branch_prefix = input("> ") - print("") - logger.info("Enter the ticketing provider from the list below: ") logger.info("jira") logger.info("redmine") @@ -79,7 +83,12 @@ def add_ticketing_instance(): logger.error(f"Provider {provider} not supported!") sys.exit(1) + logger.info("Enter the branch prefix (e.g ABC for feature/ABC-123_ticket): ") + branch_prefix = input("> ") + print("") + if provider == "jira": + project_prefix = f"{branch_prefix}-" else: project_prefix = None @@ -97,10 +106,13 @@ def add_ticketing_instance(): url=base_url, token=api_token ) - if instance.validate_connection(): - logger.info("Connection validated!") - else: - logger.info("Failed to validate connection!") + try: + instance.validate_connection() + except ConnectionError as e: + logger.error(f"Failed to connect! {e}") + sys.exit(1) + + logger.info("Connection succesful!") ticket_cfg = TicketHostCfg( url=base_url, @@ -109,4 +121,6 @@ def add_ticketing_instance(): project_prefix=project_prefix ) - ReviewConfig.write_ticketing_data(branch_prefix, ticket_cfg) + ReviewConfig().write_ticketing_data(branch_prefix, ticket_cfg) + + logger.info("Added ticketing instance!") diff --git a/src/sc/review/review.py b/src/sc/review/review.py index b344a40..67095cf 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -11,12 +11,12 @@ from git_flow_library import GitFlowLibrary from sc_manifest_parser import ScManifest -from .exceptions import RemoteUrlNotFound, TicketIdentifierNotFound, TicketNotFound -from .review_config import ReviewConfig +from .exceptions import RemoteUrlNotFound, TicketIdentifierNotFound +from .review_config import ReviewConfig, TicketHostCfg from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory from .ticketing_instances.ticketing_instance import TicketingInstance -from .vcs_instances.vcs_factory import VcsFactory -from .vcs_instances.vcs_instance import VcsInstance +from .git_instances.git_factory import GitFactory +from .git_instances.git_instance import GitInstance logger = logging.getLogger(__name__) @@ -32,40 +32,7 @@ class CommentData: commit_author: str commit_date: datetime commit_message: str - - def generate_comment(self, formatted: bool = False) -> str: - header = [ - f"Branch: [{self.branch}]", - f"Directory: [{self.directory}]", - f"Git: [{self.remote_url}]", - ] - - review = [ - f"Review Status: [{self.review_status}]", - f"Review URL: [{self.review_url}]" if self.review_url else - f"Create Review URL: [{self.create_pr_url}]" - ] - - commit = ( - f"Last Commit: [{self.commit_sha}]", - f"Author: [{self.commit_author}]", - f"Date: [{self.commit_date}]", - "", - f"{self.commit_message}" - ) - - if formatted: - commit = ["
", *commit, "
"] - - return "\n".join( - [ - *header, - "", - *review, - "", - *commit - ] - ) + class Review: def __init__(self, top_dir: str): @@ -86,35 +53,26 @@ def run_git_command(self): identifier, ticket_num = None, None if identifier and ticket_num: - ticket_host_data = self._config.get_ticket_host_data(identifier) - ticketing_instance: TicketingInstance = TicketingInstanceFactory.create( - ticket_host_data.class_name, - ticket_host_data.url, - ticket_host_data.api_key, - ticket_host_data.username - ) + ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) - ticket_id = f"{ticket_host_data.project_prefix or ''}{ticket_num}" + ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) else: ticketing_instance = None ticket = None - vcs_instance = self._create_vcs_instance(Repo(self.top_dir).remote().url) - comment_data = self._create_comment_data(Repo(self.top_dir), vcs_instance) + git_instance = self._create_git_instance(Repo(self.top_dir).remote().url) + comment_data = self._create_comment_data(Repo(self.top_dir), git_instance) logger.info(f"Ticket URL: [{ticket.url if ticket else 'None'}]") logger.info("Ticket info: ") - print(comment_data.generate_comment()) + print(self._generate_terminal_comment(comment_data)) if self._prompt_yn("Update ticket?"): - if ticketing_instance and ticket_id: - ticketing_instance.add_comment_to_ticket( - ticket_id, comment_data.generate_comment(formatted=True)) - else: + ticket_comment = self._generate_ticket_comment(comment_data) + if not (ticketing_instance and ticket_id): ticketing_instance, ticket_id = self._prompt_ticket_selection() - ticketing_instance.add_comment_to_ticket( - ticket_id, comment_data.generate_comment(formatted=True)) + ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) def run_repo_command(self): manifest_repo = Repo(self.top_dir / '.repo' / 'manifests') @@ -131,15 +89,9 @@ def run_repo_command(self): identifier, ticket_num = None, None if identifier and ticket_num: - ticket_host_data = self._config.get_ticket_host_data(identifier) - ticketing_instance = TicketingInstanceFactory.create( - ticket_host_data.class_name, - ticket_host_data.url, - ticket_host_data.api_key, - ticket_host_data.username - ) + ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) - ticket_id = f"{ticket_host_data.project_prefix or ''}{ticket_num}" + ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) else: ticketing_instance, ticket_id = None, None @@ -150,28 +102,31 @@ def run_repo_command(self): manifest = ScManifest.from_repo_root(self.top_dir / '.repo') comments = [] for project in manifest.projects: + if project.lock_status: + continue + proj_repo = Repo(self.top_dir / project.path) - proj_vcs = self._create_vcs_instance(proj_repo.remotes[proj_repo.remote].url) + #Don't generate for projects that haven't got an upstream + if not proj_repo.active_branch.tracking_branch(): + continue + + proj_git = self._create_git_instance(proj_repo.remotes[project.remote].url) comment_data = self._create_comment_data( - proj_repo, proj_vcs) + proj_repo, proj_git) comments.append(comment_data) - manifest_vcs = self._create_vcs_instance(manifest_repo.remote().url) + manifest_git = self._create_git_instance(manifest_repo.remote().url) comment_data = self._create_comment_data( - manifest_repo, manifest_vcs) + manifest_repo, manifest_git) comments.append(comment_data) - multi_repo_comment = self._generate_multi_comment(comments) - print(multi_repo_comment) + print(self._generate_combined_terminal_comment(comments)) if self._prompt_yn("Update ticket?"): - if ticketing_instance and ticket_id: - ticketing_instance.add_comment_to_ticket( - ticket_id, multi_repo_comment) - else: + ticket_comment = self._generate_combined_ticket_comment(comments) + if not (ticketing_instance and ticket_id): ticketing_instance, ticket_id = self._prompt_ticket_selection() - ticketing_instance.add_comment_to_ticket( - ticket_id, multi_repo_comment) + ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) def _match_branch( self, @@ -200,32 +155,32 @@ def _match_branch( f"Branch {branch_name} doesn't match any ticketing instances! " f"Found instances {', '.join(host_identifiers)}") - def _create_vcs_instance(self, remote_url: str) -> VcsInstance: - vcs_url_patterns = self._config.get_vcs_patterns() + def _create_git_instance(self, remote_url: str) -> GitInstance: + git_url_patterns = self._config.get_git_patterns() try: remote_pattern = self._match_remote_url( - remote_url, vcs_url_patterns) + remote_url, git_url_patterns) except RemoteUrlNotFound as e: raise RemoteUrlNotFound( - e + f"\nRemotes patterns found: {', '.join(vcs_url_patterns)}" + str(e) + f"\nRemotes patterns found: {', '.join(git_url_patterns)}" ) - vcs_data = self._config.get_vcs_data(remote_pattern) - return VcsFactory.create( - vcs_data.provider, - token=vcs_data.token, - base_url=vcs_data.url + git_data = self._config.get_git_data(remote_pattern) + return GitFactory.create( + git_data.provider, + token=git_data.token, + base_url=git_data.url ) def _match_remote_url( self, remote_url: str, - vcs_patterns: Iterable[str] + git_patterns: Iterable[str] ) -> str: - """Match the remote url to a pattern in the vcs config. + """Match the remote url to a pattern in the git instance config. Args: remote_url (str): The remote url of the git repository. - vcs_patterns (Iterable[str]): An iterable of patterns to check against. + git_patterns (Iterable[str]): An iterable of patterns to check against. Raises: RemoteUrlNotFound: Raised when the remote url matches no patterns. @@ -233,7 +188,7 @@ def _match_remote_url( Returns: str: The matched pattern. """ - for pattern in vcs_patterns: + for pattern in git_patterns: if pattern in remote_url: return pattern raise RemoteUrlNotFound(f"{remote_url} doesn't match any patterns!") @@ -257,24 +212,24 @@ def _get_target_branch(self, directory: Path, source_branch: str) -> str: def _prompt_yn(self, msg: str) -> bool: return input(f"{msg} (y/n): ").strip().lower() == 'y' - def _create_comment_data(self, repo: Repo, vcs_instance: VcsInstance) -> CommentData: + def _create_comment_data(self, repo: Repo, git_instance: GitInstance) -> CommentData: branch_name = repo.active_branch.name - repo_slug = self._get_repo_slug(repo.remote().url) - pr = vcs_instance.get_pull_request(repo_slug, branch_name) + repo_slug = self._get_repo_slug(repo.remotes[0].url) + pr = git_instance.get_pull_request(repo_slug, branch_name) - target_branch = self._get_target_branch(self.top_dir, branch_name) - create_pr_url = vcs_instance.get_create_pr_url( + target_branch = self._get_target_branch(repo.working_dir, branch_name) + create_pr_url = git_instance.get_create_pr_url( repo_slug, branch_name, target_branch) commit = repo.head.commit - review_status = pr.status if pr else "Not Created" + review_status = str(pr.status) if pr else "Not Created" review_url = pr.url if pr else None return CommentData( branch=branch_name, directory=repo.working_dir, - remote_url=repo.remote().url, + remote_url=repo.remotes[0].url, review_status=review_status, review_url=review_url, create_pr_url=create_pr_url, @@ -284,13 +239,12 @@ def _create_comment_data(self, repo: Repo, vcs_instance: VcsInstance) -> Comment commit_message=commit.message.strip() ) - def _generate_combined_comment( - comments: list[CommentData], formatted: bool = False) -> str: - parts = [] - for c in comments: - parts.append(f"{c.generate_comment(formatted=formatted)}\n") - return "\n".join(parts) - + def _load_ticketing(self, identifier: str) -> tuple[TicketingInstance, TicketHostCfg]: + cfg = self._config.get_ticket_host_data(identifier) + inst = TicketingInstanceFactory.create( + cfg.provider, cfg.url, cfg.api_key, cfg.cert) + return inst, cfg + def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: """Prompt the user to select a ticketing instance and enter a ticket number. @@ -310,20 +264,91 @@ def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: input_id = input("> ") - ticket_instance_conf = ticket_conf.get(input_id) - if not ticket_instance_conf: - raise TicketIdentifierNotFound(f"No prefix matches: {input_id}") - - ticketing_instance = TicketingInstanceFactory.create( - ticket_instance_conf.class_name, - ticket_instance_conf.url, - ticket_instance_conf.api_key, - ticket_instance_conf.username - ) + ticketing_instance, ticketing_conf = self._load_ticketing(input_id) logger.info("Please enter your ticket number:") input_num = input("> ") - ticket_id = f"{ticket_instance_conf.project_prefix or ''}{input_num}" + ticket_id = f"{ticketing_conf.project_prefix or ''}{input_num}" return ticketing_instance, ticket_id + + def _generate_combined_terminal_comment(self, comments: list[CommentData]) -> str: + return "\n\n".join(self._generate_terminal_comment(c) for c in comments) + + def _generate_combined_ticket_comment(self, comments: list[CommentData]) -> str: + return "\n\n".join(self._generate_ticket_comment(c) for c in comments) + + def _generate_terminal_comment(self, data: CommentData) -> str: + """Generate the information for one repo to be displayed in the terminal. + + Args: + data (CommentData): The data collated from one repo. + + Returns: + str: Information from one repo to be displayed in the terminal. + """ + def c(code, text): + return f"\033[{code}m{text}\033[0m" + + header = [ + f"Branch: [{data.branch}]", + f"Directory: [{data.directory}]", + f"Git: [{data.remote_url}]", + ] + + logger.info(f"Data: {str(data)}") + if data.review_url: + review_status = f"Review Status: [{c('32', data.review_status)}]" + review_link = f"Review URL: [{c('32', data.review_url)}]" + else: + review_status = f"Review Status: [{c('31', data.review_status)}]" + review_link = f"Create Review URL: [{c('33', data.create_pr_url)}]" + + review = [review_status, review_link] + + commit = ( + f"Last Commit: [{data.commit_sha}]", + f"Author: [{data.commit_author}]", + f"Date: [{data.commit_date}]", + "", + f"{data.commit_message}" + ) + + return "\n".join([*header, "", *review, "", *commit]) + + def _generate_ticket_comment(self, data: CommentData) -> str: + """Generate the information for one repo formatted for a ticket comment. + + Args: + data (CommentData): The data collated for one repo. + + Returns: + str: A formatted ticket comment. + """ + header = [ + f"Branch: [{data.branch}]", + f"Directory: [{data.directory}]", + f"Git: [{data.remote_url}]", + ] + + if data.review_url: + review_status = f"Review Status: [{data.review_status}]" + review_link = f"Review URL: [{data.review_url}]" + else: + review_status = f"Review Status: [{data.review_status}]" + review_link = f"Create Review URL: [{data.create_pr_url}]" + + review = [review_status, review_link] + + commit = ( + "
",
+            f"Last Commit: [{data.commit_sha}]",
+            f"Author: [{data.commit_author}]",
+            f"Date: [{data.commit_date}]",
+            "",
+            f"{data.commit_message}"
+            "
" + ) + + return "\n".join([*header, "", *review, "", *commit]) diff --git a/src/sc/review/review_config.py b/src/sc/review/review_config.py index 68beab2..24a4bc4 100644 --- a/src/sc/review/review_config.py +++ b/src/sc/review/review_config.py @@ -1,19 +1,19 @@ +from pydantic import BaseModel, ConfigDict, ValidationError from sc.config_manager import ConfigManager -from pydantic import BaseModel, ConfigDict, ValidationError - class TicketHostCfg(BaseModel): model_config = ConfigDict(extra='forbid') url: str - class_name: str + provider: str username: str | None = None api_key: str project_prefix: str | None = None description: str | None = None + cert: str | None = None -class VcsCfg(BaseModel): +class GitInstanceCfg(BaseModel): model_config = ConfigDict(extra='forbid') url: str | None = None @@ -23,7 +23,7 @@ class VcsCfg(BaseModel): class ReviewConfig: def __init__(self): self._ticket_config = ConfigManager('ticketing_instances') - self._vcs_config = ConfigManager('vcs_instances') + self._git_config = ConfigManager('git_instances') def get_ticketing_config(self) -> dict[str, TicketHostCfg]: return {k: TicketHostCfg(**v) for k,v in self._ticket_config.get_config().items()} @@ -34,27 +34,27 @@ def get_ticket_host_ids(self) -> set[str]: def get_ticket_host_data(self, identifier: str) -> TicketHostCfg: data = self._ticket_config.get_config().get(identifier) if not data: - raise KeyError(f"VCS config doesn't contain entry for {identifier}") + raise KeyError(f"Git instance config doesn't contain entry for {identifier}") try: return TicketHostCfg(**data) except ValidationError as e: raise ValueError(f"Invalid config for {identifier}: {e}") def write_ticketing_data(self, branch_prefix: str, ticket_data: TicketHostCfg): - self._ticket_config.update_config({branch_prefix: {ticket_data.model_dump()}}) + self._ticket_config.update_config({branch_prefix: ticket_data.model_dump(exclude_none=True)}) - def get_vcs_patterns(self) -> set[str]: - return self._vcs_config.get_config().keys() + def get_git_patterns(self) -> set[str]: + return self._git_config.get_config().keys() - def get_vcs_data(self, url_pattern: str) -> VcsCfg: - data = self._vcs_config.get_config().get(url_pattern) + def get_git_data(self, url_pattern: str) -> GitInstanceCfg: + data = self._git_config.get_config().get(url_pattern) if not data: - raise KeyError(f"VCS config doesn't contain entry for {url_pattern}") + raise KeyError(f"Git config doesn't contain entry for {url_pattern}") try: - return VcsCfg(**data) + return GitInstanceCfg(**data) except ValidationError as e: raise ValueError(f"Invalid config for {url_pattern}: {e}") - def write_vcs_data(self, pattern: str, vcs_config: VcsCfg): - self._vcs_config.update_config({pattern: vcs_config.model_dump()}) + def write_git_data(self, pattern: str, git_config: GitInstanceCfg): + self._git_config.update_config({pattern: git_config.model_dump(exclude_none=True)}) \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/__init__.py b/src/sc/review/ticketing_instances/__init__.py index dd1eee6..e69de29 100644 --- a/src/sc/review/ticketing_instances/__init__.py +++ b/src/sc/review/ticketing_instances/__init__.py @@ -1,3 +0,0 @@ -from .jira_instance import JiraInstance -from .redmine_instance import RedmineInstance -from .downloader import Downloader \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py index 8e1dd0e..19bcd2c 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -78,6 +78,21 @@ def __del__(self): @property def engine(self) -> str: return 'jira' + + def validate_connection(self) -> bool: + """Check that the Jira instance and credentials are valid.""" + try: + # Simple lightweight call that requires authentication + self._instance.myself() + return True + + except JIRAError as e: + if e.status_code in (401, 403): + raise ConnectionError("Invalid Jira credentials or insufficient permissions.") from e + raise ConnectionAbortedError(f"Jira API error: HTTP {e.status_code}") from e + + except RequestException as e: + raise ConnectionError("Unable to reach Jira server.") from e def create_ticket(self, project_key: str, issue_type: str, title: str, **kwargs): @@ -192,3 +207,13 @@ def delete_ticket(self, ticket_id: str): ticket_id (str, optional): The id of the ticket to delete. Defaults to None. """ self._instance.issue(ticket_id).delete() + + def _set_cert(self, cert_path): + cert = self._get_cert(cert_path) + self._default_jira_setup['client_cert'] = cert + + def _get_cert(self, cert_path): + cert_file = self._get_file(cert_path, + output_location='/tmp/', + obfuscate=True) + return cert_file diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/redmine_instance.py index 69ee2cd..619f619 100644 --- a/src/sc/review/ticketing_instances/redmine_instance.py +++ b/src/sc/review/ticketing_instances/redmine_instance.py @@ -3,8 +3,8 @@ from urllib3.exceptions import InsecureRequestWarning from redminelib import Redmine -from redminelib.exceptions import ForbiddenError, ResourceNotFoundError, AuthError -from requests.exceptions import SSLError +from redminelib.exceptions import BaseRedmineError, ForbiddenError, ResourceNotFoundError, AuthError +from requests.exceptions import RequestException, SSLError from ..exceptions import TicketNotFound, TicketingInstanceUnreachable, PermissionsError from .ticketing_instance import TicketingInstance @@ -38,7 +38,21 @@ def engine(self) -> str: return 'redmine' def validate_connection(self) -> bool: - pass + """Check if the Redmine instance and API key are valid.""" + try: + # Minimal authenticated request + self._instance.auth() # redminelib verifies credentials + return True + + except (AuthError, ForbiddenError) as e: + ConnectionError("Invalid Redmine API key or insufficient permssions.") + + except BaseRedmineError as e: + raise ConnectionError(f"Redmine API error: {e}") + + except RequestException as e: + raise ConnectionError("Failed to reach Redmine server.") from e + def create_ticket(self, project_name: str, title: str, **kwargs) -> Ticket: """Create a ticket on the redmine instance diff --git a/src/sc/review/ticketing_instances/ticket_instance_factory.py b/src/sc/review/ticketing_instances/ticket_instance_factory.py index 98ef761..2447909 100644 --- a/src/sc/review/ticketing_instances/ticket_instance_factory.py +++ b/src/sc/review/ticketing_instances/ticket_instance_factory.py @@ -19,9 +19,10 @@ def create( provider: str, url: str, token: str, + cert: str | None = None ) -> TicketingInstance: try: - return cls._registry[provider](url=url, password=token) + return cls._registry[provider](url=url, password=token, cert=cert) except KeyError: raise TicketIdentifierNotFound( f"Provider {provider} doesn't match any ticketing instance!") diff --git a/src/sc/review/ticketing_instances/ticketing_instance.py b/src/sc/review/ticketing_instances/ticketing_instance.py index b42887e..2e6f53d 100644 --- a/src/sc/review/ticketing_instances/ticketing_instance.py +++ b/src/sc/review/ticketing_instances/ticketing_instance.py @@ -27,6 +27,18 @@ def url(self): def engine(self) -> str: pass + @abstractmethod + def validate_connection(self) -> bool: + """Abstract Method: + Validate connection to the ticketing instance. + + Raises: + ConnectionError: If the connection is unsuccessful. + + Returns: + bool: True if connection is successful. + """ + @abstractmethod def create_ticket(self) -> Ticket: """Abstract Method: @@ -136,3 +148,14 @@ def _version_check(self, required_version: str, if int(instance_bug) > int(required_bug): return 'later' return 'earlier' + + def _get_file(self, + file_location: str, + output_location: str = None, + obfuscate: bool = False): + filepath = abspath(file_location) if exists(file_location) else None + if filepath is None and self._downloader: + filepath = self._downloader.download(file_location, + output_dir=output_location, + obfuscate=obfuscate) + return filepath diff --git a/src/sc/review/vcs_instances/pull_request.py b/src/sc/review/vcs_instances/pull_request.py deleted file mode 100644 index 8f3c79d..0000000 --- a/src/sc/review/vcs_instances/pull_request.py +++ /dev/null @@ -1,12 +0,0 @@ -from dataclasses import dataclass -from enum import Enum - -class PRStatus(Enum): - OPEN = "open" - CLOSED = "closed" - MERGED = "merged" - -@dataclass -class PullRequest: - url: str - status: PRStatus \ No newline at end of file diff --git a/src/sc/review_cli.py b/src/sc/review_cli.py index b4f558c..3d56290 100644 --- a/src/sc/review_cli.py +++ b/src/sc/review_cli.py @@ -8,12 +8,15 @@ def cli(): @cli.command() def review(): + """Add commit/PR information to your ticket.""" main.review() @cli.command() -def add_vcs_instance(): - main.add_vcs_instance() +def add_git_instance(): + """Add a VCS instance for sc review.""" + main.add_git_instance() @cli.command() def add_ticketing_instance(): + """Add a ticketing instance for sc review.""" main.add_ticketing_instance() \ No newline at end of file From 98c1abd82a61b740725f4a60d801965dfc8767cf Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Tue, 18 Nov 2025 14:14:59 +0000 Subject: [PATCH 03/12] Changes rom copilot --- src/sc/review/main.py | 4 +- src/sc/review/review.py | 2 +- src/sc/review/review_config.py | 2 +- .../ticketing_instances/jira_instance.py | 25 ++++------- .../ticketing_instances/redmine_instance.py | 45 ++++++++++--------- src/sc/review/ticketing_instances/ticket.py | 2 +- .../ticketing_instances/ticketing_instance.py | 11 ----- 7 files changed, 38 insertions(+), 53 deletions(-) diff --git a/src/sc/review/main.py b/src/sc/review/main.py index 9389c4f..5ca2239 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -36,7 +36,7 @@ def add_git_instance(): if provider == "github": url = "http://api.github.com" - logger.info("Enter a pattern for to identify Git from remote url: ") + logger.info("Enter a pattern to identify Git from remote url: ") logger.info( "E.G. github.com for all github instances or " "github.com/org for a particular organisation") @@ -112,7 +112,7 @@ def add_ticketing_instance(): logger.error(f"Failed to connect! {e}") sys.exit(1) - logger.info("Connection succesful!") + logger.info("Connection successful!") ticket_cfg = TicketHostCfg( url=base_url, diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 67095cf..85f8133 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -347,7 +347,7 @@ def _generate_ticket_comment(self, data: CommentData) -> str: f"Author: [{data.commit_author}]", f"Date: [{data.commit_date}]", "", - f"{data.commit_message}" + f"{data.commit_message}", "" ) diff --git a/src/sc/review/review_config.py b/src/sc/review/review_config.py index 24a4bc4..2d321d8 100644 --- a/src/sc/review/review_config.py +++ b/src/sc/review/review_config.py @@ -34,7 +34,7 @@ def get_ticket_host_ids(self) -> set[str]: def get_ticket_host_data(self, identifier: str) -> TicketHostCfg: data = self._ticket_config.get_config().get(identifier) if not data: - raise KeyError(f"Git instance config doesn't contain entry for {identifier}") + raise KeyError(f"Ticket instance config doesn't contain entry for {identifier}") try: return TicketHostCfg(**data) except ValidationError as e: diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py index 19bcd2c..3c7d8b2 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -19,8 +19,9 @@ class JiraInstance(TicketingInstance): """A class to handle operations on Jira tickets. Args: - url (str): URL of Jira instance. - password (str): Password to authenticate with. + url (str): URL of Jira instance. + password (str): Password to authenticate with. + cert (str | None): Path to a cert file. Common KWargs: verify (bool): Whether to use ssl verification. Default True @@ -30,7 +31,7 @@ class JiraInstance(TicketingInstance): TicketingInstanceUnreachable: If the ticketing instance cannot be connected to. """ - def __init__(self, url: str, password: str, **kwargs): + def __init__(self, url: str, password: str, cert: str | None, **kwargs): self._default_jira_setup = { 'server': '', 'verify': True, @@ -44,13 +45,13 @@ def __init__(self, url: str, password: str, **kwargs): # If the user does not want to use SSL verification disable the warnings about it being insecure. if self._default_jira_setup.get('verify') is False: disable_warnings(InsecureRequestWarning) - # 9.0.0. is an arbitrary version that hasn't been release yet + # 9.0.0. is an arbitrary version that hasn't been released yet # Jira is currently readying version 3 of the api so we should be ready for it if kwargs.get('version') and self._version_check( '9.0.0', kwargs.get('version', '0.0.0')) in ('same', 'later'): self._default_jira_setup['rest_api_version'] = '3' - if kwargs.get('cert') or kwargs.get('certificate'): - self._set_cert(kwargs.get('cert', kwargs.get('certificate'))) + if cert: + self._default_jira_setup['client_cert'] = cert try: self._instance = JIRA( options=self._default_jira_setup, @@ -99,7 +100,7 @@ def create_ticket(self, project_key: str, issue_type: str, title: str, """Creates a ticket on the Jira instance Args: - project_key (str): The projects key on th Jira instance + project_key (str): The project's key on the Jira instance issue_type (str): The type of issue to raise titlr (str): Title of the ticket to raise """ @@ -207,13 +208,3 @@ def delete_ticket(self, ticket_id: str): ticket_id (str, optional): The id of the ticket to delete. Defaults to None. """ self._instance.issue(ticket_id).delete() - - def _set_cert(self, cert_path): - cert = self._get_cert(cert_path) - self._default_jira_setup['client_cert'] = cert - - def _get_cert(self, cert_path): - cert_file = self._get_file(cert_path, - output_location='/tmp/', - obfuscate=True) - return cert_file diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/redmine_instance.py index 619f619..2b53fbb 100644 --- a/src/sc/review/ticketing_instances/redmine_instance.py +++ b/src/sc/review/ticketing_instances/redmine_instance.py @@ -22,30 +22,36 @@ def __init__(self, url, password: str, **kwargs): # If the user does not want to use SSL verification disable the warnings about it being insecure. if kwargs.get('verify') is False: disable_warnings(InsecureRequestWarning) - try: - self._instance = Redmine(url, - key=self._credentials.get('password'), - requests={ - 'verify': kwargs.get('verify', True), - 'proxies': kwargs.get('proxies', {}) - }, - version=kwargs.get('version', None)) - except: - raise TicketingInstanceUnreachable(url) + self._instance = Redmine( + url, + key=self._credentials.get('password'), + requests={ + 'verify': kwargs.get('verify', True), + 'proxies': kwargs.get('proxies', {}) + }, + version=kwargs.get('version', None) + ) @property def engine(self) -> str: return 'redmine' def validate_connection(self) -> bool: - """Check if the Redmine instance and API key are valid.""" + """Check if the Redmine instance and API key are valid. + + Raises: + ConnectionError: If the connection is invalid. + + Returns: + bool: True if connection is valid. + """ try: # Minimal authenticated request self._instance.auth() # redminelib verifies credentials return True except (AuthError, ForbiddenError) as e: - ConnectionError("Invalid Redmine API key or insufficient permssions.") + raise ConnectionError("Invalid Redmine API key or insufficient permssions.") except BaseRedmineError as e: raise ConnectionError(f"Redmine API error: {e}") @@ -72,7 +78,7 @@ def create_ticket(self, project_name: str, title: str, **kwargs) -> Ticket: except (ResourceNotFoundError, SSLError) as exception: raise TicketingInstanceUnreachable( self.url, - additional_information=''.join( + additional_info=''.join( arg for arg in exception.args)) from exception except ForbiddenError as exception: raise PermissionsError( @@ -138,8 +144,7 @@ def update_ticket(self, ticket_id, **kwargs): PermissionsError: User does not have permission to access the ticket on the instances TicketingInstanceUnreachable: The redmine instance is unreachable """ - issue_url = '{url}/issues/{ticket_number}'.format( - url=self.url, ticket_number=ticket_id) + issue_url = f'{self.url}/issues/{ticket_id}' try: self._instance.issue.update(ticket_id, **kwargs) except ResourceNotFoundError as exception: @@ -161,7 +166,8 @@ def add_comment_to_ticket(self, ticket_id: str, comment_message: str) -> None: comment_message (str): The message to add as a comment ticket_id (str): The ticket number to add the comment to. """ - ticket = self.update_ticket(ticket_id,notes=self._convert_html_colours(comment_message)) + ticket = self.update_ticket( + ticket_id, notes=self._convert_html_colours(comment_message)) return ticket def _convert_html_colours(self, string: str) -> str: @@ -189,15 +195,14 @@ def delete_ticket(self, ticket_id: str): PermissionsError: User does not have permission to access the ticket on the instances TicketingInstanceUnreachable: The redmine instance is unreachable """ - issue_url = '{url}/issues/{ticket_number}'.format( - url=self._instance.url, ticket_number=self.ticket_id) + issue_url = f'{self.url}/issues/{ticket_id}' try: - self._instance.issue.delete(self.ticket_id) + self._instance.issue.delete(ticket_id) except ResourceNotFoundError as exception: raise TicketNotFound(issue_url) from exception except ForbiddenError as exception: raise PermissionsError( - '{url}/issues/{ticket_number}'.format(url=self._url), + f'{self._instance.url}/issues/{ticket_id}', 'Please contact the dev-support team') from exception except SSLError as exception: raise TicketingInstanceUnreachable( diff --git a/src/sc/review/ticketing_instances/ticket.py b/src/sc/review/ticketing_instances/ticket.py index 89de025..b300bcf 100644 --- a/src/sc/review/ticketing_instances/ticket.py +++ b/src/sc/review/ticketing_instances/ticket.py @@ -43,4 +43,4 @@ def url(self) -> str: return self._contents.get('url') def get(self, undeclared_property): - return self._contents['contents'].get(undeclared_property) + return self._contents.get(undeclared_property) diff --git a/src/sc/review/ticketing_instances/ticketing_instance.py b/src/sc/review/ticketing_instances/ticketing_instance.py index 2e6f53d..988d7e3 100644 --- a/src/sc/review/ticketing_instances/ticketing_instance.py +++ b/src/sc/review/ticketing_instances/ticketing_instance.py @@ -148,14 +148,3 @@ def _version_check(self, required_version: str, if int(instance_bug) > int(required_bug): return 'later' return 'earlier' - - def _get_file(self, - file_location: str, - output_location: str = None, - obfuscate: bool = False): - filepath = abspath(file_location) if exists(file_location) else None - if filepath is None and self._downloader: - filepath = self._downloader.download(file_location, - output_dir=output_location, - obfuscate=obfuscate) - return filepath From a7d47a69df054f5cc3cda29442c570d59bab7fef Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Mon, 24 Nov 2025 14:46:54 +0000 Subject: [PATCH 04/12] BWDO-520 changes after review --- .../pull_request.py => core/code_review.py} | 6 +- .../{git_instances => core}/git_instance.py | 18 +-- src/sc/review/{ => core}/review_config.py | 15 ++- .../{ticketing_instances => core}/ticket.py | 0 .../ticketing_instance.py | 0 src/sc/review/exceptions.py | 2 +- src/sc/review/factories/__init__.py | 2 + .../git_factory.py | 7 +- .../ticket_instance_factory.py | 5 +- src/sc/review/git_instances/__init__.py | 2 + .../review/git_instances/github_instance.py | 27 +++-- .../review/git_instances/gitlab_instance.py | 26 ++--- src/sc/review/main.py | 28 ++--- src/sc/review/review.py | 109 ++++++++---------- src/sc/review/ticketing_instances/__init__.py | 2 + .../ticketing_instances/jira_instance.py | 7 +- .../ticketing_instances/redmine_instance.py | 11 +- 17 files changed, 123 insertions(+), 144 deletions(-) rename src/sc/review/{git_instances/pull_request.py => core/code_review.py} (74%) rename src/sc/review/{git_instances => core}/git_instance.py (73%) rename src/sc/review/{ => core}/review_config.py (98%) rename src/sc/review/{ticketing_instances => core}/ticket.py (100%) rename src/sc/review/{ticketing_instances => core}/ticketing_instance.py (100%) create mode 100644 src/sc/review/factories/__init__.py rename src/sc/review/{git_instances => factories}/git_factory.py (76%) rename src/sc/review/{ticketing_instances => factories}/ticket_instance_factory.py (83%) create mode 100644 src/sc/review/git_instances/__init__.py diff --git a/src/sc/review/git_instances/pull_request.py b/src/sc/review/core/code_review.py similarity index 74% rename from src/sc/review/git_instances/pull_request.py rename to src/sc/review/core/code_review.py index d5691ca..642f25c 100644 --- a/src/sc/review/git_instances/pull_request.py +++ b/src/sc/review/core/code_review.py @@ -1,7 +1,7 @@ from dataclasses import dataclass from enum import Enum -class PRStatus(str, Enum): +class CRStatus(str, Enum): OPEN = "Open" CLOSED = "Closed" MERGED = "Merged" @@ -10,6 +10,6 @@ def __str__(self): return self.value @dataclass -class PullRequest: +class CodeReview: url: str - status: PRStatus \ No newline at end of file + status: CRStatus \ No newline at end of file diff --git a/src/sc/review/git_instances/git_instance.py b/src/sc/review/core/git_instance.py similarity index 73% rename from src/sc/review/git_instances/git_instance.py rename to src/sc/review/core/git_instance.py index 44420d2..46dfeb8 100644 --- a/src/sc/review/git_instances/git_instance.py +++ b/src/sc/review/core/git_instance.py @@ -1,13 +1,13 @@ from abc import ABC, abstractmethod -from .pull_request import PullRequest +from .code_review import CodeReview class GitInstance(ABC): @abstractmethod 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: @@ -18,18 +18,18 @@ def validate_connection(self) -> bool: Returns: bool: True if connection is successful. - """ + """ pass @abstractmethod - def get_pull_request(self, repo: str, source_branch: str) -> PullRequest: + def get_code_review(self, repo: str, source_branch: str) -> CodeReview: pass @abstractmethod - def get_create_pr_url( - self, - repo: str, - source_branch: str, + def get_create_cr_url( + self, + repo: str, + source_branch: str, target_branch: str = "develop" - ): + ) -> str: pass \ No newline at end of file diff --git a/src/sc/review/review_config.py b/src/sc/review/core/review_config.py similarity index 98% rename from src/sc/review/review_config.py rename to src/sc/review/core/review_config.py index 2d321d8..ea05db2 100644 --- a/src/sc/review/review_config.py +++ b/src/sc/review/core/review_config.py @@ -24,13 +24,13 @@ class ReviewConfig: def __init__(self): self._ticket_config = ConfigManager('ticketing_instances') self._git_config = ConfigManager('git_instances') - + def get_ticketing_config(self) -> dict[str, TicketHostCfg]: return {k: TicketHostCfg(**v) for k,v in self._ticket_config.get_config().items()} - + def get_ticket_host_ids(self) -> set[str]: return self._ticket_config.get_config().keys() - + def get_ticket_host_data(self, identifier: str) -> TicketHostCfg: data = self._ticket_config.get_config().get(identifier) if not data: @@ -39,13 +39,13 @@ def get_ticket_host_data(self, identifier: str) -> TicketHostCfg: return TicketHostCfg(**data) except ValidationError as e: raise ValueError(f"Invalid config for {identifier}: {e}") - + def write_ticketing_data(self, branch_prefix: str, ticket_data: TicketHostCfg): self._ticket_config.update_config({branch_prefix: ticket_data.model_dump(exclude_none=True)}) - + def get_git_patterns(self) -> set[str]: return self._git_config.get_config().keys() - + def get_git_data(self, url_pattern: str) -> GitInstanceCfg: data = self._git_config.get_config().get(url_pattern) if not data: @@ -54,7 +54,6 @@ def get_git_data(self, url_pattern: str) -> GitInstanceCfg: return GitInstanceCfg(**data) except ValidationError as e: raise ValueError(f"Invalid config for {url_pattern}: {e}") - + def write_git_data(self, pattern: str, git_config: GitInstanceCfg): self._git_config.update_config({pattern: git_config.model_dump(exclude_none=True)}) - \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/ticket.py b/src/sc/review/core/ticket.py similarity index 100% rename from src/sc/review/ticketing_instances/ticket.py rename to src/sc/review/core/ticket.py diff --git a/src/sc/review/ticketing_instances/ticketing_instance.py b/src/sc/review/core/ticketing_instance.py similarity index 100% rename from src/sc/review/ticketing_instances/ticketing_instance.py rename to src/sc/review/core/ticketing_instance.py diff --git a/src/sc/review/exceptions.py b/src/sc/review/exceptions.py index cc753cf..9a7f0ce 100644 --- a/src/sc/review/exceptions.py +++ b/src/sc/review/exceptions.py @@ -41,4 +41,4 @@ class TicketIdentifierNotFound(ReviewException): class RemoteUrlNotFound(ReviewException): """Raised when remote url matches no patterns in the config. - """ \ No newline at end of file + """ diff --git a/src/sc/review/factories/__init__.py b/src/sc/review/factories/__init__.py new file mode 100644 index 0000000..dfd2402 --- /dev/null +++ b/src/sc/review/factories/__init__.py @@ -0,0 +1,2 @@ +from .git_factory import GitFactory +from . ticket_instance_factory import TicketingInstanceFactory \ No newline at end of file diff --git a/src/sc/review/git_instances/git_factory.py b/src/sc/review/factories/git_factory.py similarity index 76% rename from src/sc/review/git_instances/git_factory.py rename to src/sc/review/factories/git_factory.py index 7555f5b..0d4e71b 100644 --- a/src/sc/review/git_instances/git_factory.py +++ b/src/sc/review/factories/git_factory.py @@ -1,7 +1,6 @@ -from .github_instance import GithubInstance -from .gitlab_instance import GitlabInstance -from .git_instance import GitInstance +from ..git_instances import GithubInstance, GitlabInstance +from ..core.git_instance import GitInstance class GitFactory: _registry = { @@ -18,4 +17,4 @@ def create(cls, name: str, token: str, base_url: str | None) -> GitInstance: try: return cls._registry[name.lower()](token=token, base_url=base_url) except KeyError: - raise ValueError(f"Provider name {name} doesn't match any VCS instance!") \ No newline at end of file + raise ValueError(f"Provider name {name} doesn't match any VCS instance!") diff --git a/src/sc/review/ticketing_instances/ticket_instance_factory.py b/src/sc/review/factories/ticket_instance_factory.py similarity index 83% rename from src/sc/review/ticketing_instances/ticket_instance_factory.py rename to src/sc/review/factories/ticket_instance_factory.py index 2447909..b62b480 100644 --- a/src/sc/review/ticketing_instances/ticket_instance_factory.py +++ b/src/sc/review/factories/ticket_instance_factory.py @@ -1,7 +1,6 @@ from sc.review.exceptions import TicketIdentifierNotFound -from .jira_instance import JiraInstance -from .redmine_instance import RedmineInstance -from .ticketing_instance import TicketingInstance +from ..ticketing_instances import JiraInstance, RedmineInstance +from ..core.ticketing_instance import TicketingInstance class TicketingInstanceFactory: _registry = { diff --git a/src/sc/review/git_instances/__init__.py b/src/sc/review/git_instances/__init__.py new file mode 100644 index 0000000..e8c8830 --- /dev/null +++ b/src/sc/review/git_instances/__init__.py @@ -0,0 +1,2 @@ +from .github_instance import GithubInstance +from .gitlab_instance import GitlabInstance \ No newline at end of file diff --git a/src/sc/review/git_instances/github_instance.py b/src/sc/review/git_instances/github_instance.py index abb95c1..323fe0e 100644 --- a/src/sc/review/git_instances/github_instance.py +++ b/src/sc/review/git_instances/github_instance.py @@ -1,7 +1,7 @@ import requests -from .pull_request import PRStatus, PullRequest -from .git_instance import GitInstance +from ..core.code_review import CRStatus, CodeReview +from ..core.git_instance import GitInstance class GithubInstance(GitInstance): def __init__(self, token: str, base_url: str | None): @@ -12,7 +12,7 @@ def _headers(self) -> dict: "Accept": "application/vnd.github.v3+json", "Authorization": f"Bearer {self.token}" } - + def validate_connection(self) -> bool: url = f"{self.base_url}/user" try: @@ -27,31 +27,30 @@ def validate_connection(self) -> bool: except requests.exceptions.ConnectionError as e: raise ConnectionError("Network connection to GitHub failed.") from e - def get_pull_request(self, repo: str, source_branch: str) -> PullRequest | None: + def get_code_review(self, repo: str, source_branch: str) -> CodeReview | None: url = f"{self.base_url}/repos/{repo}/pulls" owner = repo.split("/")[0] params = {"state": "all", "head": f"{owner}:{source_branch}"} r = requests.get(url, headers=self._headers(), params=params, timeout=10) r.raise_for_status() prs = r.json() - print(r.json()) if not prs: return None pr = prs[0] # GitHub marks merged PRs as state="closed", merged=True if pr.get("merged"): - status = PRStatus.MERGED + status = CRStatus.MERGED elif pr["state"] == "open": - status = PRStatus.OPEN + status = CRStatus.OPEN else: - status = PRStatus.CLOSED - - return PullRequest(url=pr["html_url"], status=status) + status = CRStatus.CLOSED + + return CodeReview(url=pr["html_url"], status=status) - def get_create_pr_url( - self, - repo: str, - source_branch: str, + def get_create_cr_url( + self, + repo: str, + source_branch: str, target_branch: str = "develop" ) -> str: return f"https://github.com/{repo}/compare/{target_branch}...{source_branch}" \ No newline at end of file diff --git a/src/sc/review/git_instances/gitlab_instance.py b/src/sc/review/git_instances/gitlab_instance.py index d29af2c..df6689c 100644 --- a/src/sc/review/git_instances/gitlab_instance.py +++ b/src/sc/review/git_instances/gitlab_instance.py @@ -1,16 +1,16 @@ import requests import urllib.parse -from .pull_request import PullRequest, PRStatus -from .git_instance import GitInstance +from ..core.code_review import CodeReview, CRStatus +from ..core.git_instance import GitInstance class GitlabInstance(GitInstance): def __init__(self, token: str, base_url: str): super().__init__(token, base_url) - + def _headers(self) -> dict[str, str]: return {"Private-Token": self.token} - + def validate_connection(self) -> bool: url = f"{self.base_url}/api/v4/user" try: @@ -25,8 +25,7 @@ def validate_connection(self) -> bool: except requests.exceptions.ConnectionError as e: raise ConnectionError("Network connection to GitLab failed.") from e - - def get_pull_request(self, repo: str, source_branch: str) -> PullRequest: + def get_code_review(self, repo: str, source_branch: str) -> CodeReview: safe_repo = urllib.parse.quote(repo, safe='') url = f"{self.base_url}/api/v4/projects/{safe_repo}/merge_requests" params = {"state": "all", "source_branch": source_branch} @@ -39,19 +38,18 @@ def get_pull_request(self, repo: str, source_branch: str) -> PullRequest: state = pr["state"] if state == "merged": - status = PRStatus.MERGED + status = CRStatus.MERGED elif state == "opened": - status = PRStatus.OPEN + status = CRStatus.OPEN else: - status = PRStatus.CLOSED + status = CRStatus.CLOSED + + return CodeReview(url=pr["web_url"], status=status) - return PullRequest(url=pr["web_url"], status=status) - - def get_create_pr_url(self, repo, source_branch, target_branch = "develop"): - safe_repo = urllib.parse.quote(repo, safe="") + def get_create_cr_url(self, repo, source_branch, target_branch = "develop"): params = { "merge_request[source_branch]": source_branch, "merge_request[target_branch]": target_branch, } query = urllib.parse.urlencode(params) - return f"{self.base_url}/{safe_repo}/-/merge_requests/new?{query}" \ No newline at end of file + return f"{self.base_url}/{repo}/-/merge_requests/new?{query}" diff --git a/src/sc/review/main.py b/src/sc/review/main.py index 5ca2239..0de3115 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -7,9 +7,9 @@ from repo_library import RepoLibrary from .exceptions import ReviewException from .review import Review -from .review_config import ReviewConfig, TicketHostCfg, GitInstanceCfg -from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory -from .git_instances.git_factory import GitFactory +from .core.review_config import ReviewConfig, TicketHostCfg, GitInstanceCfg +from .factories.ticket_instance_factory import TicketingInstanceFactory +from .factories.git_factory import GitFactory logger = logging.getLogger(__name__) @@ -51,8 +51,8 @@ def add_git_instance(): pattern = url.replace("https://", "").replace("http://", "") else: logger.error("Provider matches none in the list!") - - + + logger.info("Enter your api token: ") api_key = input("> ") print("") @@ -64,9 +64,9 @@ def add_git_instance(): except ConnectionError as e: logger.error(f"Failed to connect! {e}") sys.exit(1) - + logger.info("Connection validated!") - + git_cfg = GitInstanceCfg(url=url, token=api_key, provider=provider) ReviewConfig().write_git_data(pattern, git_cfg) @@ -82,13 +82,13 @@ def add_ticketing_instance(): if provider not in ("jira", "redmine"): logger.error(f"Provider {provider} not supported!") sys.exit(1) - + logger.info("Enter the branch prefix (e.g ABC for feature/ABC-123_ticket): ") branch_prefix = input("> ") print("") if provider == "jira": - + project_prefix = f"{branch_prefix}-" else: project_prefix = None @@ -102,7 +102,7 @@ def add_ticketing_instance(): print("") instance = TicketingInstanceFactory.create( - provider=provider, + provider=provider, url=base_url, token=api_token ) @@ -115,12 +115,12 @@ def add_ticketing_instance(): logger.info("Connection successful!") ticket_cfg = TicketHostCfg( - url=base_url, - provider=provider, - api_key=api_token, + url=base_url, + provider=provider, + api_key=api_token, project_prefix=project_prefix ) - + ReviewConfig().write_ticketing_data(branch_prefix, ticket_cfg) logger.info("Added ticketing instance!") diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 85f8133..2523fdb 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -12,11 +12,11 @@ from sc_manifest_parser import ScManifest from .exceptions import RemoteUrlNotFound, TicketIdentifierNotFound -from .review_config import ReviewConfig, TicketHostCfg -from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory -from .ticketing_instances.ticketing_instance import TicketingInstance -from .git_instances.git_factory import GitFactory -from .git_instances.git_instance import GitInstance +from .core.review_config import ReviewConfig, TicketHostCfg +from .factories.ticket_instance_factory import TicketingInstanceFactory +from .core.ticketing_instance import TicketingInstance +from .factories.git_factory import GitFactory +from .core.git_instance import GitInstance logger = logging.getLogger(__name__) @@ -32,7 +32,7 @@ class CommentData: commit_author: str commit_date: datetime commit_message: str - + class Review: def __init__(self, top_dir: str): @@ -41,60 +41,46 @@ def __init__(self, top_dir: str): self._config = ReviewConfig() def run_git_command(self): - ticket_host_ids = self._config.get_ticket_host_ids() branch_name = Repo(self.top_dir).active_branch.name try: - identifier, ticket_num = self._match_branch( - branch_name, - ticket_host_ids - ) - except TicketIdentifierNotFound as e: - logger.warning(e) - identifier, ticket_num = None, None - - if identifier and ticket_num: + identifier, ticket_num = self._match_branch(branch_name) ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) - + ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) - else: + except TicketIdentifierNotFound as e: + logger.warning(e) ticketing_instance = None + ticket_id = None ticket = None - + git_instance = self._create_git_instance(Repo(self.top_dir).remote().url) comment_data = self._create_comment_data(Repo(self.top_dir), git_instance) logger.info(f"Ticket URL: [{ticket.url if ticket else 'None'}]") logger.info("Ticket info: ") print(self._generate_terminal_comment(comment_data)) - + if self._prompt_yn("Update ticket?"): ticket_comment = self._generate_ticket_comment(comment_data) if not (ticketing_instance and ticket_id): ticketing_instance, ticket_id = self._prompt_ticket_selection() ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) - + def run_repo_command(self): manifest_repo = Repo(self.top_dir / '.repo' / 'manifests') - ticket_host_ids = self._config.get_ticket_host_ids() - try: - identifier, ticket_num = self._match_branch( - manifest_repo.active_branch.name, - ticket_host_ids - ) - except TicketIdentifierNotFound as e: - logger.warning(e) - identifier, ticket_num = None, None - - if identifier and ticket_num: + identifier, ticket_num = self._match_branch(manifest_repo.active_branch.name) ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) - + ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) - else: - ticketing_instance, ticket_id = None, None + except TicketIdentifierNotFound as e: + logger.warning(e) + ticketing_instance = None + ticket_id = None + ticket = None logger.info(f"Ticket URL: [{ticket.url if ticket else ''}]") logger.info("Ticket info: ") @@ -118,7 +104,7 @@ def run_repo_command(self): manifest_git = self._create_git_instance(manifest_repo.remote().url) comment_data = self._create_comment_data( manifest_repo, manifest_git) - comments.append(comment_data) + comments.append(comment_data) print(self._generate_combined_terminal_comment(comments)) @@ -127,17 +113,12 @@ def run_repo_command(self): if not (ticketing_instance and ticket_id): ticketing_instance, ticket_id = self._prompt_ticket_selection() ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) - - def _match_branch( - self, - branch_name: str, - host_identifiers: Iterable[str] - ) -> tuple[str, str]: + + def _match_branch(self, branch_name: str) -> tuple[str, str]: """Match the branch to an identifier in the config. Args: branch_name (str): The current branch name. - host_identifiers (Iterable[str]): An iterable of ticket host identifiers. Raises: TicketIdentifierNotFound: Raised when the branch doesn't match any @@ -145,7 +126,8 @@ def _match_branch( Returns: tuple[str, str]: (matched_identifier, ticket_number) - """ + """ + host_identifiers = self._config.get_ticket_host_ids() for identifier in host_identifiers: # Matches the identifier, followed by - or _, followed by a number if m := re.search(fr'{identifier}[-_]?(\d+)', branch_name): @@ -154,7 +136,7 @@ def _match_branch( raise TicketIdentifierNotFound( f"Branch {branch_name} doesn't match any ticketing instances! " f"Found instances {', '.join(host_identifiers)}") - + def _create_git_instance(self, remote_url: str) -> GitInstance: git_url_patterns = self._config.get_git_patterns() try: @@ -170,7 +152,7 @@ def _create_git_instance(self, remote_url: str) -> GitInstance: token=git_data.token, base_url=git_data.url ) - + def _match_remote_url( self, remote_url: str, @@ -187,44 +169,44 @@ def _match_remote_url( Returns: str: The matched pattern. - """ + """ for pattern in git_patterns: if pattern in remote_url: return pattern raise RemoteUrlNotFound(f"{remote_url} doesn't match any patterns!") - + def _get_repo_slug(self, remote_url: str) -> str: """Return the repository slug (e.g. "org/repo") from a remote url.""" if remote_url.startswith("git@"): slug = remote_url.split(":", 1)[1] else: slug = parse.urlparse(remote_url).path.lstrip("/") - + if slug.endswith(".git"): slug = slug[:-4] - + return slug def _get_target_branch(self, directory: Path, source_branch: str) -> str: base = GitFlowLibrary.get_branch_base(source_branch, directory) return base if base else GitFlowLibrary.get_develop_branch(directory) - + def _prompt_yn(self, msg: str) -> bool: return input(f"{msg} (y/n): ").strip().lower() == 'y' - + def _create_comment_data(self, repo: Repo, git_instance: GitInstance) -> CommentData: branch_name = repo.active_branch.name repo_slug = self._get_repo_slug(repo.remotes[0].url) - pr = git_instance.get_pull_request(repo_slug, branch_name) + cr = git_instance.get_code_review(repo_slug, branch_name) target_branch = self._get_target_branch(repo.working_dir, branch_name) - create_pr_url = git_instance.get_create_pr_url( + create_pr_url = git_instance.get_create_cr_url( repo_slug, branch_name, target_branch) - + commit = repo.head.commit - review_status = str(pr.status) if pr else "Not Created" - review_url = pr.url if pr else None + review_status = str(cr.status) if cr else "Not Created" + review_url = cr.url if cr else None return CommentData( branch=branch_name, @@ -238,13 +220,13 @@ def _create_comment_data(self, repo: Repo, git_instance: GitInstance) -> Comment commit_date=commit.committed_datetime, commit_message=commit.message.strip() ) - + def _load_ticketing(self, identifier: str) -> tuple[TicketingInstance, TicketHostCfg]: cfg = self._config.get_ticket_host_data(identifier) inst = TicketingInstanceFactory.create( cfg.provider, cfg.url, cfg.api_key, cfg.cert) return inst, cfg - + def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: """Prompt the user to select a ticketing instance and enter a ticket number. @@ -261,7 +243,7 @@ def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: logger.info("PREFIX --- INSTANCE URL --- DESCRIPTION") for id, conf in ticket_conf.items(): logger.info(f"{id} --- {conf.url} --- {conf.description or ''}") - + input_id = input("> ") ticketing_instance, ticketing_conf = self._load_ticketing(input_id) @@ -287,7 +269,7 @@ def _generate_terminal_comment(self, data: CommentData) -> str: Returns: str: Information from one repo to be displayed in the terminal. - """ + """ def c(code, text): return f"\033[{code}m{text}\033[0m" @@ -297,7 +279,6 @@ def c(code, text): f"Git: [{data.remote_url}]", ] - logger.info(f"Data: {str(data)}") if data.review_url: review_status = f"Review Status: [{c('32', data.review_status)}]" review_link = f"Review URL: [{c('32', data.review_url)}]" @@ -316,7 +297,7 @@ def c(code, text): ) return "\n".join([*header, "", *review, "", *commit]) - + def _generate_ticket_comment(self, data: CommentData) -> str: """Generate the information for one repo formatted for a ticket comment. @@ -325,7 +306,7 @@ def _generate_ticket_comment(self, data: CommentData) -> str: Returns: str: A formatted ticket comment. - """ + """ header = [ f"Branch: [{data.branch}]", f"Directory: [{data.directory}]", diff --git a/src/sc/review/ticketing_instances/__init__.py b/src/sc/review/ticketing_instances/__init__.py index e69de29..33ad7ae 100644 --- a/src/sc/review/ticketing_instances/__init__.py +++ b/src/sc/review/ticketing_instances/__init__.py @@ -0,0 +1,2 @@ +from .jira_instance import JiraInstance +from .redmine_instance import RedmineInstance \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py index 3c7d8b2..5ba2d7e 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -11,9 +11,8 @@ from jira.exceptions import JIRAError from ..exceptions import PermissionsError, TicketingInstanceUnreachable, TicketNotFound -from .ticket import Ticket -from .ticketing_instance import TicketingInstance - +from ..core.ticket import Ticket +from ..core.ticketing_instance import TicketingInstance class JiraInstance(TicketingInstance): """A class to handle operations on Jira tickets. @@ -79,7 +78,7 @@ def __del__(self): @property def engine(self) -> str: return 'jira' - + def validate_connection(self) -> bool: """Check that the Jira instance and credentials are valid.""" try: diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/redmine_instance.py index 2b53fbb..fb98c30 100644 --- a/src/sc/review/ticketing_instances/redmine_instance.py +++ b/src/sc/review/ticketing_instances/redmine_instance.py @@ -7,9 +7,8 @@ from requests.exceptions import RequestException, SSLError from ..exceptions import TicketNotFound, TicketingInstanceUnreachable, PermissionsError -from .ticketing_instance import TicketingInstance -from .ticket import Ticket - +from ..core.ticketing_instance import TicketingInstance +from ..core.ticket import Ticket class RedmineInstance(TicketingInstance): """ @@ -35,10 +34,10 @@ def __init__(self, url, password: str, **kwargs): @property def engine(self) -> str: return 'redmine' - + def validate_connection(self) -> bool: """Check if the Redmine instance and API key are valid. - + Raises: ConnectionError: If the connection is invalid. @@ -52,7 +51,7 @@ def validate_connection(self) -> bool: except (AuthError, ForbiddenError) as e: raise ConnectionError("Invalid Redmine API key or insufficient permssions.") - + except BaseRedmineError as e: raise ConnectionError(f"Redmine API error: {e}") From f59d29c309e5e3b5dab7b2000ada9fea2a972b44 Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Tue, 9 Dec 2025 10:41:02 +0000 Subject: [PATCH 05/12] Added copyright and basic auth --- src/sc/clone_cli.py | 29 +++- src/sc/review/core/__init__.py | 0 src/sc/review/core/code_review.py | 14 ++ src/sc/review/core/git_instance.py | 14 ++ src/sc/review/core/review_config.py | 19 ++- src/sc/review/core/ticket.py | 14 +- src/sc/review/core/ticketing_instance.py | 83 ++--------- src/sc/review/exceptions.py | 14 ++ src/sc/review/factories/git_factory.py | 13 ++ .../factories/ticket_instance_factory.py | 56 +++++-- .../review/git_instances/github_instance.py | 14 ++ .../review/git_instances/gitlab_instance.py | 14 ++ src/sc/review/main.py | 38 ++++- src/sc/review/review.py | 14 ++ .../ticketing_instances/jira_instance.py | 137 ++++++------------ .../ticketing_instances/redmine_instance.py | 93 +++--------- src/sc/review_cli.py | 16 +- src/sc/sc_logging.py | 17 ++- 18 files changed, 334 insertions(+), 265 deletions(-) create mode 100644 src/sc/review/core/__init__.py diff --git a/src/sc/clone_cli.py b/src/sc/clone_cli.py index 06d55e9..7633939 100755 --- a/src/sc/clone_cli.py +++ b/src/sc/clone_cli.py @@ -1,4 +1,19 @@ #!/usr/bin/env python3 +# +# 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 click from sc.clone import SCClone @@ -17,22 +32,22 @@ def cli(): @click.option('-v', '--verify', is_flag=True, help='RDK projects only, run post-sync-hooks without prompts.') @click.pass_context def clone( - ctx, - project: str | None, - directory: str | None, + ctx, + project: str | None, + directory: str | None, rev: str | None, no_tags: bool, - manifest: str | None, + manifest: str | None, force: bool, verify: bool, ): """Clone groups of repositories from a config.""" if project: SCClone().clone( - project_name=project, - directory=directory, + project_name=project, + directory=directory, rev=rev, - no_tags=no_tags, + no_tags=no_tags, manifest=manifest, force_overwrite=force, verify=verify, diff --git a/src/sc/review/core/__init__.py b/src/sc/review/core/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/sc/review/core/code_review.py b/src/sc/review/core/code_review.py index 642f25c..fe38fac 100644 --- a/src/sc/review/core/code_review.py +++ b/src/sc/review/core/code_review.py @@ -1,3 +1,17 @@ +# 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 diff --git a/src/sc/review/core/git_instance.py b/src/sc/review/core/git_instance.py index 46dfeb8..742fff9 100644 --- a/src/sc/review/core/git_instance.py +++ b/src/sc/review/core/git_instance.py @@ -1,3 +1,17 @@ +# 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 diff --git a/src/sc/review/core/review_config.py b/src/sc/review/core/review_config.py index ea05db2..b966265 100644 --- a/src/sc/review/core/review_config.py +++ b/src/sc/review/core/review_config.py @@ -1,3 +1,19 @@ +# 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 typing import Literal + from pydantic import BaseModel, ConfigDict, ValidationError from sc.config_manager import ConfigManager @@ -7,8 +23,9 @@ class TicketHostCfg(BaseModel): url: str provider: str - username: str | None = None api_key: str + username: str | None = None + auth_type: Literal["token", "basic"] = "token" project_prefix: str | None = None description: str | None = None cert: str | None = None diff --git a/src/sc/review/core/ticket.py b/src/sc/review/core/ticket.py index b300bcf..3a2d71c 100644 --- a/src/sc/review/core/ticket.py +++ b/src/sc/review/core/ticket.py @@ -1,4 +1,16 @@ -#!/usr/bin/env python3 +# 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 Ticket(): diff --git a/src/sc/review/core/ticketing_instance.py b/src/sc/review/core/ticketing_instance.py index 988d7e3..3e4436b 100644 --- a/src/sc/review/core/ticketing_instance.py +++ b/src/sc/review/core/ticketing_instance.py @@ -1,27 +1,26 @@ -#!/usr/bin/env python3 +# 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 os.path import abspath, exists from .ticket import Ticket - class TicketingInstance(ABC): """ A class to handle the ticket(s) that the source control commands are operating on. """ - - @abstractmethod - def __init__(self, url: str, **kwargs): - self._credentials = {} - self._url = url - if 'username' and 'password' in kwargs.keys(): - self.set_credentials(kwargs.get('username'), kwargs.get('password')) - - @property - def url(self): - return self._url - @property @abstractmethod def engine(self) -> str: @@ -65,20 +64,6 @@ def read_ticket(self, ticket_id: str) -> Ticket: """ pass - @abstractmethod - def update_ticket(self, ticket_id, **kwargs) -> Ticket: - """Abstract Method: - Updates the contents dictionary. - Should update the tickets with the changes in the kwargs. - Reads the updated ticket and returns it as a new ticket object - - Args: - ticket_id (str): The id of the ticket to update. - Returns: - ticket (Ticket): New ticket object with update applied. - """ - pass - @abstractmethod def add_comment_to_ticket(self, ticket_id: str, comment_message: str) -> Ticket: @@ -108,43 +93,3 @@ def delete_ticket(self, ticket_id: str) -> bool: result (bool): True if ticket removed successfully. """ pass - - def set_credentials(self, username: str, password: str): - """Set the username and password for in the credentials dictionary. - - Args: - username (str): Username of the user to login to the ticketing instance with - password (str): Password/API key of the user to login to the ticketing instance with - """ - self._credentials = {'username': username, 'password': password} - - def _version_check(self, required_version: str, - instance_version: str) -> str: - """Compares the versions. - - Args: - required_version (str): Version to compare against - instance_version (str): Version to check - - Returns: - str: (later, same, ealier) depending on the comparison of the versions - """ - if required_version.count('.') < 2: - required_version += '.0.0' - if instance_version.count('.') < 2: - instance_version += '.0.0' - required_major, required_minor, required_bug = required_version.split( - '.') - instance_major, instance_minor, instance_bug = instance_version.split( - '.') - if instance_version == required_version: - return 'same' - if int(instance_major) > int(required_major): - return 'later' - elif int(instance_major) == int(required_major): - if int(instance_minor) > int(required_minor): - return 'later' - elif int(instance_minor) == int(required_minor): - if int(instance_bug) > int(required_bug): - return 'later' - return 'earlier' diff --git a/src/sc/review/exceptions.py b/src/sc/review/exceptions.py index 9a7f0ce..a744a17 100644 --- a/src/sc/review/exceptions.py +++ b/src/sc/review/exceptions.py @@ -1,3 +1,17 @@ +# 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 diff --git a/src/sc/review/factories/git_factory.py b/src/sc/review/factories/git_factory.py index 0d4e71b..7ad97c9 100644 --- a/src/sc/review/factories/git_factory.py +++ b/src/sc/review/factories/git_factory.py @@ -1,3 +1,16 @@ +# 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 ..git_instances import GithubInstance, GitlabInstance from ..core.git_instance import GitInstance diff --git a/src/sc/review/factories/ticket_instance_factory.py b/src/sc/review/factories/ticket_instance_factory.py index b62b480..488d246 100644 --- a/src/sc/review/factories/ticket_instance_factory.py +++ b/src/sc/review/factories/ticket_instance_factory.py @@ -1,27 +1,59 @@ +# 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 sc.review.exceptions import TicketIdentifierNotFound from ..ticketing_instances import JiraInstance, RedmineInstance from ..core.ticketing_instance import TicketingInstance -class TicketingInstanceFactory: - _registry = { - "redmine": RedmineInstance, - "jira": JiraInstance - } - - @classmethod - def providers(cls) -> list[str]: - return list(cls._registry.keys()) +from jira import JIRA +from redminelib import Redmine +class TicketingInstanceFactory: @classmethod def create( cls, provider: str, url: str, token: str, + auth_type: str = "token", + username: str | None = None, cert: str | None = None ) -> TicketingInstance: - try: - return cls._registry[provider](url=url, password=token, cert=cert) - except KeyError: + if provider == "redmine": + instance = Redmine( + url, + key=token + ) + return RedmineInstance(instance) + + elif provider == "jira": + options = {} + if cert: + options["client_cert"] = cert + if auth_type == "token": + instance = JIRA( + server=url, + token_auth=token, + options=options + ) + elif auth_type == "basic": + instance = JIRA( + server=url, + basic_auth=(username, token), + options=options + ) + return JiraInstance(instance) + else: raise TicketIdentifierNotFound( f"Provider {provider} doesn't match any ticketing instance!") diff --git a/src/sc/review/git_instances/github_instance.py b/src/sc/review/git_instances/github_instance.py index 323fe0e..aa5bdf7 100644 --- a/src/sc/review/git_instances/github_instance.py +++ b/src/sc/review/git_instances/github_instance.py @@ -1,3 +1,17 @@ +# 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 ..core.code_review import CRStatus, CodeReview diff --git a/src/sc/review/git_instances/gitlab_instance.py b/src/sc/review/git_instances/gitlab_instance.py index df6689c..22036ae 100644 --- a/src/sc/review/git_instances/gitlab_instance.py +++ b/src/sc/review/git_instances/gitlab_instance.py @@ -1,3 +1,17 @@ +# 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 import urllib.parse diff --git a/src/sc/review/main.py b/src/sc/review/main.py index 0de3115..5287af8 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -1,3 +1,17 @@ +# 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 getpass import logging from pathlib import Path @@ -88,10 +102,26 @@ def add_ticketing_instance(): print("") if provider == "jira": - project_prefix = f"{branch_prefix}-" + + logger.info("Auth type:") + logger.info("token") + logger.info("basic") + auth_type = input("> ") + print("") + + if auth_type not in ("token", "basic"): + logger.error(f"Auth type {auth_type} not supported!") + sys.exit(1) + + if auth_type == "basic": + logger.info("Username:") + username = input("> ") + print("") + else: project_prefix = None + username = None logger.info("Enter the base URL: ") base_url = input("> ") @@ -104,7 +134,9 @@ def add_ticketing_instance(): instance = TicketingInstanceFactory.create( provider=provider, url=base_url, - token=api_token + token=api_token, + auth_type=auth_type, + username=username ) try: instance.validate_connection() @@ -118,6 +150,8 @@ def add_ticketing_instance(): url=base_url, provider=provider, api_key=api_token, + username=username, + auth_type=auth_type, project_prefix=project_prefix ) diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 2523fdb..56fc411 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -1,3 +1,17 @@ +# 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 collections.abc import Iterable from dataclasses import dataclass from datetime import datetime diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py index 5ba2d7e..a94f328 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -1,11 +1,20 @@ -#!/usr/bin/env python3 +# 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 json.decoder import JSONDecodeError -import os from requests import post from requests.exceptions import InvalidURL, RequestException -from urllib3 import disable_warnings -from urllib3.exceptions import InsecureRequestWarning from jira import JIRA from jira.exceptions import JIRAError @@ -30,50 +39,11 @@ class JiraInstance(TicketingInstance): TicketingInstanceUnreachable: If the ticketing instance cannot be connected to. """ - def __init__(self, url: str, password: str, cert: str | None, **kwargs): - self._default_jira_setup = { - 'server': '', - 'verify': True, - 'rest_api_version': '2', - } - super().__init__(url, password=password, **kwargs) - self._default_jira_setup['server'] = url - self._default_jira_setup['verify'] = kwargs.get('verify', True) - if isinstance(kwargs.get('extra_setup_options'), dict): - self._default_jira_setup.update(kwargs.get('extra_setup_options')) - # If the user does not want to use SSL verification disable the warnings about it being insecure. - if self._default_jira_setup.get('verify') is False: - disable_warnings(InsecureRequestWarning) - # 9.0.0. is an arbitrary version that hasn't been released yet - # Jira is currently readying version 3 of the api so we should be ready for it - if kwargs.get('version') and self._version_check( - '9.0.0', kwargs.get('version', '0.0.0')) in ('same', 'later'): - self._default_jira_setup['rest_api_version'] = '3' - if cert: - self._default_jira_setup['client_cert'] = cert - try: - self._instance = JIRA( - options=self._default_jira_setup, - token_auth=self._credentials.get('password'), - proxies=kwargs.get('proxies', None)) - except AttributeError as e: - raise TicketingInstanceUnreachable(url, - additional_info=''.join( - arg for arg in e.args)) - except JIRAError as e: - raise TicketingInstanceUnreachable(e.url, - additional_info=e.status_code) - except JSONDecodeError: - raise TicketingInstanceUnreachable( - url, additional_info='Certificate Response Error') - except InvalidURL as e: - raise TicketingInstanceUnreachable(url, - additional_info=''.join( - arg for arg in e.args)) - - def __del__(self): - if self._default_jira_setup.get('client_cert'): - os.remove(self._default_jira_setup.get('client_cert')) + def __init__( + self, + instance: JIRA + ): + self._instance = instance @property def engine(self) -> str: @@ -94,22 +64,6 @@ def validate_connection(self) -> bool: except RequestException as e: raise ConnectionError("Unable to reach Jira server.") from e - def create_ticket(self, project_key: str, issue_type: str, title: str, - **kwargs): - """Creates a ticket on the Jira instance - - Args: - project_key (str): The project's key on the Jira instance - issue_type (str): The type of issue to raise - titlr (str): Title of the ticket to raise - """ - project_dict = {'project': {'key': project_key}} - kwargs.update(project_dict) - kwargs.update({'issuetype': issue_type, 'summary': title}) - new_issue_id = self._instance.create_issue(fields=kwargs).key - ticket = self.read_ticket(new_issue_id) - return ticket - def read_ticket(self, ticket_id: str) -> Ticket: """Reads the contents of the ticket and puts the dictionary in to contents @@ -120,8 +74,8 @@ def read_ticket(self, ticket_id: str) -> Ticket: issue_contents = self._instance.issue(ticket_id).raw['fields'] except JIRAError as e: if 'permission' in e.text: - raise PermissionsError(e.url, - 'Please contact the dev-support team') + raise PermissionsError( + e.url, 'Please contact the dev-support team') else: raise TicketNotFound(e.url) try: @@ -136,8 +90,7 @@ def read_ticket(self, ticket_id: str) -> Ticket: status = issue_contents['status'].get('name') target_version = ', '.join([version.get('name') for version in issue_contents.get('fixVersions')]) title = issue_contents.get('summary', '') - ticket_url = '{instance_url}/browse/{ticket_id}'.format( - instance_url=self.url, ticket_id=ticket_id) + ticket_url = f'{self.url}/browse/{ticket_id}' ticket = Ticket(ticket_url, assignee=assignee, author=author, @@ -150,19 +103,6 @@ def read_ticket(self, ticket_id: str) -> Ticket: url=ticket_url) return ticket - def update_ticket(self, ticket_id: str, **kwargs): - """Updates the ticket with the new value for the fields based on the kwargs - - Args: - ticket_id (str): The id of the ticket to update. Defaults to None. - """ - try: - self._instance.issue(ticket_id).update(**kwargs) - except JIRAError as e: - raise TicketNotFound(e.url) - ticket = self.read_ticket(ticket_id) - return ticket - def add_comment_to_ticket(self, ticket_id: str, comment_message: str): """Adds a comment to the ticket @@ -172,7 +112,8 @@ def add_comment_to_ticket(self, ticket_id: str, comment_message: str): """ try: comment = self._convert_from_html(comment_message) - ticket = self.update_ticket(ticket_id, comment=comment) + ticket = self._instance.add_comment(ticket_id, comment=comment) + ticket = self._instance.up except TicketNotFound: # Some workflows do not accept the above method for adding a comment as it technically rewrites the whole ticket try: @@ -180,17 +121,29 @@ def add_comment_to_ticket(self, ticket_id: str, comment_message: str): content = {'body': comment} post('{url}/rest/api/2/issue/{ticket_id}/comment'.format( url=self.url, ticket_id=ticket_id), - json=content, - headers={ - "Authorization": f"Bearer {self._credentials.get('password')}" - }, - cert=self._default_jira_setup.get('client_cert'), - proxies=self._instance._session.proxies) + json=content, + headers={ + "Authorization": f"Bearer {self._credentials.get('password')}" + }, + cert=self._default_jira_setup.get('client_cert'), + proxies=self._instance._session.proxies) ticket = self.read_ticket(ticket_id) except RequestException: raise TicketNotFound(self.url) return ticket + def _update_ticket(self, ticket_id: str, **kwargs): + """Updates the ticket with the new value for the fields based on the kwargs + + Args: + ticket_id (str): The id of the ticket to update. Defaults to None. + """ + try: + self._instance.issue(ticket_id).update(**kwargs) + except JIRAError as e: + raise TicketNotFound(e.url) + ticket = self.read_ticket(ticket_id) + return ticket def _convert_from_html(self, string: str) -> str: string = string.replace('

str: @@ -58,35 +58,6 @@ def validate_connection(self) -> bool: except RequestException as e: raise ConnectionError("Failed to reach Redmine server.") from e - - def create_ticket(self, project_name: str, title: str, **kwargs) -> Ticket: - """Create a ticket on the redmine instance - - Args: - project_name (str): The project to create the ticket under - title (str): The title of the ticket - - Raises: - PermissionsError: User does not have permission to access the ticket on the instances - TicketingInstanceUnreachable: The redmine instance is unreachable - """ - try: - new_ticket = self._instance.issue.create(project_id=project_name, - subject=title, - **kwargs) - except (ResourceNotFoundError, SSLError) as exception: - raise TicketingInstanceUnreachable( - self.url, - additional_info=''.join( - arg for arg in exception.args)) from exception - except ForbiddenError as exception: - raise PermissionsError( - '{url}/{project}'.format(url=self.url, project=project_name), - 'Please contact the dev-support team') - ticket_id = new_ticket - ticket = self.read_ticket(ticket_id) - return ticket - def read_ticket(self, ticket_id: str) -> Ticket: """Read the information from a ticket and put it's contents in this object contents dict @@ -135,7 +106,7 @@ def read_ticket(self, ticket_id: str) -> Ticket: target_version=target_version) return ticket - def update_ticket(self, ticket_id, **kwargs): + def update_ticket(self, ticket_id: str, **kwargs): """Writes the changed fields from the kwargs, back to the ticket Raises: @@ -182,29 +153,3 @@ def _convert_html_colours(self, string: str) -> str: string = string.replace('">', '}') string = string.replace('

', r'%') return string - - def delete_ticket(self, ticket_id: str): - """Delete a ticket from the redmine instance - - Args: - ticket_id (str, optional): The ticket number to delete. Defaults to None. - - Raises: - TicketNotFound: Ticket not found on the redmine instance - PermissionsError: User does not have permission to access the ticket on the instances - TicketingInstanceUnreachable: The redmine instance is unreachable - """ - issue_url = f'{self.url}/issues/{ticket_id}' - try: - self._instance.issue.delete(ticket_id) - except ResourceNotFoundError as exception: - raise TicketNotFound(issue_url) from exception - except ForbiddenError as exception: - raise PermissionsError( - f'{self._instance.url}/issues/{ticket_id}', - 'Please contact the dev-support team') from exception - except SSLError as exception: - raise TicketingInstanceUnreachable( - issue_url, - additional_info=''.join(str(arg) for arg in exception.args)) - return True diff --git a/src/sc/review_cli.py b/src/sc/review_cli.py index 3d56290..3035581 100644 --- a/src/sc/review_cli.py +++ b/src/sc/review_cli.py @@ -1,6 +1,20 @@ +# 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 click -from .review import main +from .review import main @click.group() def cli(): diff --git a/src/sc/sc_logging.py b/src/sc/sc_logging.py index 289c4e1..dc5a47a 100644 --- a/src/sc/sc_logging.py +++ b/src/sc/sc_logging.py @@ -1,6 +1,20 @@ +# 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 logging -from rich.logging import RichHandler +from rich.logging import RichHandler APP_LOGGER_NAME = "sc" @@ -37,4 +51,3 @@ def format(self, record): else: self._style._fmt = self.DEFAULT_FMT return super().format(record) - \ No newline at end of file From 10adb4dfec03a10ea1a4eefc3248e2f14207da5f Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Thu, 11 Dec 2025 18:16:20 +0000 Subject: [PATCH 06/12] BWDO-520 review --- src/sc/review/core/ticket.py | 54 +++--------- src/sc/review/core/ticketing_instance.py | 27 ------ src/sc/review/exceptions.py | 2 +- .../factories/ticket_instance_factory.py | 5 +- src/sc/review/review.py | 15 +++- .../ticketing_instances/jira_instance.py | 83 +++++++------------ .../ticketing_instances/redmine_instance.py | 66 +++++++-------- 7 files changed, 90 insertions(+), 162 deletions(-) diff --git a/src/sc/review/core/ticket.py b/src/sc/review/core/ticket.py index 3a2d71c..e463393 100644 --- a/src/sc/review/core/ticket.py +++ b/src/sc/review/core/ticket.py @@ -11,48 +11,16 @@ # 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 +@dataclass class Ticket(): - - def __init__(self, ticket_url: str, **kwargs): - self._url = ticket_url - self._contents = kwargs - - @property - def author(self): - return self._contents.get('author') - - @property - def assignee(self): - return self._contents.get('assignee') - - @property - def comments(self): - return self._contents.get('comments') - - @property - def contents(self): - return self._contents - - @property - def id(self): - return self._contents.get('id') - - @property - def status(self): - return self._contents.get('status') - - @property - def target_version(self): - return self._contents.get('target_version') - - @property - def title(self) -> str: - return self._contents.get('title') - - @property - def url(self) -> str: - return self._contents.get('url') - - def get(self, undeclared_property): - return self._contents.get(undeclared_property) + url: str + author: str | None = None + assignee: str | None = None + comments: str | None = None + id: str | None = None + status: str | None = None + target_version: str | None = None + title: str | None = None + url: str | None = None diff --git a/src/sc/review/core/ticketing_instance.py b/src/sc/review/core/ticketing_instance.py index 3e4436b..62cdc05 100644 --- a/src/sc/review/core/ticketing_instance.py +++ b/src/sc/review/core/ticketing_instance.py @@ -38,20 +38,6 @@ def validate_connection(self) -> bool: bool: True if connection is successful. """ - @abstractmethod - def create_ticket(self) -> Ticket: - """Abstract Method: - Create a ticket on the ticketing instance. - Read it's contents return the new ticket object - - Returns: - ticket (Ticket): The newly created ticket object - """ - # create ticket - # read tickets contents - # return ticket object - pass - @abstractmethod def read_ticket(self, ticket_id: str) -> Ticket: """Abstract Method: @@ -80,16 +66,3 @@ def add_comment_to_ticket(self, ticket_id: str, ticket (Ticket): New ticket object with comment added """ pass - - @abstractmethod - def delete_ticket(self, ticket_id: str) -> bool: - """Abstract Method: - Should remove the ticket from the instance - - Args: - ticket_id (str, optional): The id of the ticket to remove. - - Returns: - result (bool): True if ticket removed successfully. - """ - pass diff --git a/src/sc/review/exceptions.py b/src/sc/review/exceptions.py index a744a17..c05f612 100644 --- a/src/sc/review/exceptions.py +++ b/src/sc/review/exceptions.py @@ -45,7 +45,7 @@ class PermissionsError(ReviewException): """ def __init__(self, resource: str, resolution_message: str = ''): super().__init__( - f'You do not have permission to access {resource}.\n{resolution_message}' + f'You do not have permission to access {resource}\n{resolution_message}' ) class TicketIdentifierNotFound(ReviewException): diff --git a/src/sc/review/factories/ticket_instance_factory.py b/src/sc/review/factories/ticket_instance_factory.py index 488d246..1964c30 100644 --- a/src/sc/review/factories/ticket_instance_factory.py +++ b/src/sc/review/factories/ticket_instance_factory.py @@ -11,6 +11,7 @@ # 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 urllib3 from sc.review.exceptions import TicketIdentifierNotFound from ..ticketing_instances import JiraInstance, RedmineInstance @@ -31,9 +32,11 @@ def create( cert: str | None = None ) -> TicketingInstance: if provider == "redmine": + urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) instance = Redmine( url, - key=token + key=token, + requests={'verify': False} ) return RedmineInstance(instance) diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 56fc411..e793881 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -202,8 +202,11 @@ def _get_repo_slug(self, remote_url: str) -> str: return slug def _get_target_branch(self, directory: Path, source_branch: str) -> str: - base = GitFlowLibrary.get_branch_base(source_branch, directory) - return base if base else GitFlowLibrary.get_develop_branch(directory) + if GitFlowLibrary.is_gitflow_enabled(directory): + base = GitFlowLibrary.get_branch_base(source_branch, directory) + return base if base else GitFlowLibrary.get_develop_branch(directory) + else: + return "develop" def _prompt_yn(self, msg: str) -> bool: return input(f"{msg} (y/n): ").strip().lower() == 'y' @@ -238,7 +241,13 @@ def _create_comment_data(self, repo: Repo, git_instance: GitInstance) -> Comment def _load_ticketing(self, identifier: str) -> tuple[TicketingInstance, TicketHostCfg]: cfg = self._config.get_ticket_host_data(identifier) inst = TicketingInstanceFactory.create( - cfg.provider, cfg.url, cfg.api_key, cfg.cert) + provider=cfg.provider, + url=cfg.url, + token=cfg.api_key, + auth_type=cfg.auth_type, + username=cfg.username, + cert=cfg.cert + ) return inst, cfg def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py index a94f328..15f76a2 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -11,15 +11,12 @@ # 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 json.decoder import JSONDecodeError -from requests import post -from requests.exceptions import InvalidURL, RequestException +from requests.exceptions import RequestException from jira import JIRA from jira.exceptions import JIRAError -from ..exceptions import PermissionsError, TicketingInstanceUnreachable, TicketNotFound +from ..exceptions import PermissionsError, TicketNotFound from ..core.ticket import Ticket from ..core.ticketing_instance import TicketingInstance @@ -52,7 +49,6 @@ def engine(self) -> str: def validate_connection(self) -> bool: """Check that the Jira instance and credentials are valid.""" try: - # Simple lightweight call that requires authentication self._instance.myself() return True @@ -71,37 +67,39 @@ def read_ticket(self, ticket_id: str) -> Ticket: ticket_id (str): The id of the ticket to read. Defaults to None """ try: - issue_contents = self._instance.issue(ticket_id).raw['fields'] + issue = self._instance.issue(ticket_id) except JIRAError as e: if 'permission' in e.text: raise PermissionsError( e.url, 'Please contact the dev-support team') else: raise TicketNotFound(e.url) - try: - assignee = issue_contents['assignee'].get('name') - except (KeyError, AttributeError): - assignee = None - author = issue_contents['creator'].get('name') - try: - comments = issue_contents['comment'].get('comments') - except (KeyError, AttributeError): - comments = None - status = issue_contents['status'].get('name') - target_version = ', '.join([version.get('name') for version in issue_contents.get('fixVersions')]) - title = issue_contents.get('summary', '') - ticket_url = f'{self.url}/browse/{ticket_id}' - ticket = Ticket(ticket_url, - assignee=assignee, - author=author, - comments=comments, - contents=issue_contents, - id=ticket_id, - status=status, - target_version=target_version, - title=title, - url=ticket_url) - return ticket + + f = issue.fields + assignee = getattr(f.assignee, "name", None) + author = getattr(f.reporter, "name", None) + + comments = getattr(f.comment, "comments", None) + + status = f.status.name + title = f.summary + + versions = getattr(f, "fixVersions", []) + target_version = ", ".join(v.name for v in versions) + + base_url = self._instance._options["server"] + ticket_url = f'{base_url}/browse/{ticket_id}' + + return Ticket( + url=ticket_url, + assignee=assignee, + author=author, + comments=comments, + id=ticket_id, + status=status, + target_version=target_version, + title=title, + ) def add_comment_to_ticket(self, ticket_id: str, comment_message: str): """Adds a comment to the ticket @@ -110,27 +108,8 @@ def add_comment_to_ticket(self, ticket_id: str, comment_message: str): comment_message (str): The body of the comment ticket_id (str, optional): The ticket id to add the comment to. Defaults to None. """ - try: - comment = self._convert_from_html(comment_message) - ticket = self._instance.add_comment(ticket_id, comment=comment) - ticket = self._instance.up - except TicketNotFound: - # Some workflows do not accept the above method for adding a comment as it technically rewrites the whole ticket - try: - # Try a post request directly to the add comment REST api instead - content = {'body': comment} - post('{url}/rest/api/2/issue/{ticket_id}/comment'.format( - url=self.url, ticket_id=ticket_id), - json=content, - headers={ - "Authorization": f"Bearer {self._credentials.get('password')}" - }, - cert=self._default_jira_setup.get('client_cert'), - proxies=self._instance._session.proxies) - ticket = self.read_ticket(ticket_id) - except RequestException: - raise TicketNotFound(self.url) - return ticket + comment = self._convert_from_html(comment_message) + return self._instance.add_comment(ticket_id, comment=comment) def _update_ticket(self, ticket_id: str, **kwargs): """Updates the ticket with the new value for the fields based on the kwargs diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/redmine_instance.py index 11285fe..3de50c7 100644 --- a/src/sc/review/ticketing_instances/redmine_instance.py +++ b/src/sc/review/ticketing_instances/redmine_instance.py @@ -13,6 +13,7 @@ # limitations under the License. from redminelib import Redmine +from redminelib.resources import Issue from redminelib.exceptions import BaseRedmineError, ForbiddenError, ResourceNotFoundError, AuthError from requests.exceptions import RequestException, SSLError @@ -45,8 +46,7 @@ def validate_connection(self) -> bool: bool: True if connection is valid. """ try: - # Minimal authenticated request - self._instance.auth() # redminelib verifies credentials + self._instance.auth() return True except (AuthError, ForbiddenError) as e: @@ -68,43 +68,39 @@ def read_ticket(self, ticket_id: str) -> Ticket: PermissionsError: User does not have permission to access the ticket on the instances TicketingInstanceUnreachable: The redmine instance is unreachable """ - ticket_url = self.url + '/issues/' + ticket_id + ticket_url = self._instance.url + '/issues/' + ticket_id try: - issue = self._instance.issue.get(ticket_id, include=['journals']) - except ResourceNotFoundError as exception: - raise TicketNotFound(ticket_url) from exception - except (AuthError, ForbiddenError) as exception: + issue: Issue = self._instance.issue.get(ticket_id, include=['journals']) + except ResourceNotFoundError as e: + raise TicketNotFound(ticket_url) from e + except (AuthError, ForbiddenError) as e: raise PermissionsError( ticket_url, - 'Please contact the dev-support team') from exception - except SSLError as exception: + 'Please contact the dev-support team') from e + except SSLError as e: raise TicketingInstanceUnreachable( ticket_url, - additional_info=''.join(str(arg) for arg in exception.args)) - issue_contents = dict((k, v) for k, v in list(issue)) - author = issue_contents['author'].get('name') - try: - assignee = issue_contents['assigned_to'].get('name') - except KeyError: - assignee = None - comments = issue_contents.get('journals') - status = issue_contents['status'].get('name') - try: - target_version = issue_contents['fixed_version'].get('name') - except KeyError: - target_version = None - title = issue_contents.get('subject') - ticket = Ticket(ticket_url, - author=author, - assignee=assignee, - comments=comments, - contents=issue_contents, - id=ticket_id, - status=status, - title=title, - url=ticket_url, - target_version=target_version) - return ticket + additional_info=''.join(str(arg) for arg in e.args)) + + issue_contents = issue.__dict__ + + author = issue_contents.get("author", {}).get("name") + assignee = issue_contents.get("assigned_to", {}).get("name") + comments = issue_contents.get("journals") + status = issue_contents.get("status", {}).get("name") + target_version = issue_contents.get("fixed_version", {}).get("name") + title = issue_contents.get("subject") + + return Ticket( + ticket_url, + author=author, + assignee=assignee, + comments=comments, + id=ticket_id, + status=status, + title=title, + target_version=target_version + ) def update_ticket(self, ticket_id: str, **kwargs): """Writes the changed fields from the kwargs, back to the ticket @@ -114,7 +110,7 @@ def update_ticket(self, ticket_id: str, **kwargs): PermissionsError: User does not have permission to access the ticket on the instances TicketingInstanceUnreachable: The redmine instance is unreachable """ - issue_url = f'{self.url}/issues/{ticket_id}' + issue_url = f'{self._instance.url}/issues/{ticket_id}' try: self._instance.issue.update(ticket_id, **kwargs) except ResourceNotFoundError as exception: From 274f73cb6a60d4186482951b40b01da485de6694 Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Mon, 15 Dec 2025 14:32:46 +0000 Subject: [PATCH 07/12] Changes after review --- src/sc/review/review.py | 58 +++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/sc/review/review.py b/src/sc/review/review.py index e793881..2f880a3 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -21,9 +21,9 @@ from urllib import parse from git import Repo +from rich import print from git_flow_library import GitFlowLibrary - from sc_manifest_parser import ScManifest from .exceptions import RemoteUrlNotFound, TicketIdentifierNotFound from .core.review_config import ReviewConfig, TicketHostCfg @@ -58,15 +58,14 @@ def run_git_command(self): branch_name = Repo(self.top_dir).active_branch.name try: identifier, ticket_num = self._match_branch(branch_name) - ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) - - ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" - ticket = ticketing_instance.read_ticket(ticket_id) except TicketIdentifierNotFound as e: logger.warning(e) - ticketing_instance = None - ticket_id = None - ticket = None + identifier, ticket_num = self._prompt_ticket_selection() + + ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) + + ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" + ticket = ticketing_instance.read_ticket(ticket_id) git_instance = self._create_git_instance(Repo(self.top_dir).remote().url) comment_data = self._create_comment_data(Repo(self.top_dir), git_instance) @@ -86,15 +85,14 @@ def run_repo_command(self): try: identifier, ticket_num = self._match_branch(manifest_repo.active_branch.name) - ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) - - ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" - ticket = ticketing_instance.read_ticket(ticket_id) except TicketIdentifierNotFound as e: logger.warning(e) - ticketing_instance = None - ticket_id = None - ticket = None + identifier, ticket_num = self._prompt_ticket_selection() + + ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) + + ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" + ticket = ticketing_instance.read_ticket(ticket_id) logger.info(f"Ticket URL: [{ticket.url if ticket else ''}]") logger.info("Ticket info: ") @@ -124,8 +122,6 @@ def run_repo_command(self): if self._prompt_yn("Update ticket?"): ticket_comment = self._generate_combined_ticket_comment(comments) - if not (ticketing_instance and ticket_id): - ticketing_instance, ticket_id = self._prompt_ticket_selection() ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) def _match_branch(self, branch_name: str) -> tuple[str, str]: @@ -258,8 +254,8 @@ def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: in the config. Returns: - tuple[TicketingInstance, str]: The selected ticketing instance and ticket - ID. + tuple[str, str]: The selected ticketing instance identifier and ticket + number. """ ticket_conf = self._config.get_ticketing_config() logger.info("Please enter the prefix of the ticket instance:") @@ -268,15 +264,14 @@ def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: logger.info(f"{id} --- {conf.url} --- {conf.description or ''}") input_id = input("> ") - - ticketing_instance, ticketing_conf = self._load_ticketing(input_id) + while input_id not in ticket_conf.keys: + logger.info("Prefix {input_id} not found in instances.") + input_id = input("> ") logger.info("Please enter your ticket number:") input_num = input("> ") - ticket_id = f"{ticketing_conf.project_prefix or ''}{input_num}" - - return ticketing_instance, ticket_id + return input_id, input_num def _generate_combined_terminal_comment(self, comments: list[CommentData]) -> str: return "\n\n".join(self._generate_terminal_comment(c) for c in comments) @@ -291,10 +286,11 @@ def _generate_terminal_comment(self, data: CommentData) -> str: data (CommentData): The data collated from one repo. Returns: - str: Information from one repo to be displayed in the terminal. + str: Information from one repo to be displayed in the terminal + formatted with markup to be printed with rich. """ - def c(code, text): - return f"\033[{code}m{text}\033[0m" + def c(colour, text): + return f"[{colour}]{text}[/{colour}]" header = [ f"Branch: [{data.branch}]", @@ -303,11 +299,11 @@ def c(code, text): ] if data.review_url: - review_status = f"Review Status: [{c('32', data.review_status)}]" - review_link = f"Review URL: [{c('32', data.review_url)}]" + review_status = f"Review Status: [{c('green', data.review_status)}]" + review_link = f"Review URL: [{c('green', data.review_url)}]" else: - review_status = f"Review Status: [{c('31', data.review_status)}]" - review_link = f"Create Review URL: [{c('33', data.create_pr_url)}]" + review_status = f"Review Status: [{c('red', data.review_status)}]" + review_link = f"Create Review URL: [{c('yellow', data.create_pr_url)}]" review = [review_status, review_link] From 3923af5388876f8b87929325f8f6f1c038f90e26 Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Wed, 17 Dec 2025 11:10:42 +0000 Subject: [PATCH 08/12] BWDO-520 More changes after review --- src/sc/review/core/git_instance.py | 22 +++++++++- src/sc/review/core/review_config.py | 15 +++++-- src/sc/review/core/ticket.py | 3 +- src/sc/review/factories/git_factory.py | 2 +- .../factories/ticket_instance_factory.py | 31 +++++--------- .../review/git_instances/github_instance.py | 10 +++-- .../review/git_instances/gitlab_instance.py | 11 +++-- src/sc/review/review.py | 22 +++++----- .../ticketing_instances/jira_instance.py | 40 +++++++++++-------- .../ticketing_instances/redmine_instance.py | 37 ++++++++++------- 10 files changed, 112 insertions(+), 81 deletions(-) diff --git a/src/sc/review/core/git_instance.py b/src/sc/review/core/git_instance.py index 742fff9..4230f32 100644 --- a/src/sc/review/core/git_instance.py +++ b/src/sc/review/core/git_instance.py @@ -37,6 +37,15 @@ def validate_connection(self) -> bool: @abstractmethod def get_code_review(self, repo: str, source_branch: str) -> CodeReview: + """Get information about 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 @@ -46,4 +55,15 @@ def get_create_cr_url( source_branch: str, target_branch: str = "develop" ) -> str: - pass \ No newline at end of file + """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 diff --git a/src/sc/review/core/review_config.py b/src/sc/review/core/review_config.py index b966265..7be328d 100644 --- a/src/sc/review/core/review_config.py +++ b/src/sc/review/core/review_config.py @@ -43,27 +43,35 @@ def __init__(self): self._git_config = ConfigManager('git_instances') def get_ticketing_config(self) -> dict[str, TicketHostCfg]: + """Return all ticketing instance configs keyed by identifier.""" return {k: TicketHostCfg(**v) for k,v in self._ticket_config.get_config().items()} - def get_ticket_host_ids(self) -> set[str]: + def get_ticket_host_identifiers(self) -> set[str]: + """Return all configured ticketing instance identifiers.""" return self._ticket_config.get_config().keys() def get_ticket_host_data(self, identifier: str) -> TicketHostCfg: + """Return the ticketing config for a specific identifier.""" data = self._ticket_config.get_config().get(identifier) if not data: - raise KeyError(f"Ticket instance config doesn't contain entry for {identifier}") + raise KeyError( + f"Ticket instance config doesn't contain entry for {identifier}") try: return TicketHostCfg(**data) except ValidationError as e: raise ValueError(f"Invalid config for {identifier}: {e}") def write_ticketing_data(self, branch_prefix: str, ticket_data: TicketHostCfg): - self._ticket_config.update_config({branch_prefix: ticket_data.model_dump(exclude_none=True)}) + """Persist ticketing config for a branch prefix.""" + self._ticket_config.update_config( + {branch_prefix: ticket_data.model_dump(exclude_none=True)}) def get_git_patterns(self) -> set[str]: + """Return all configured git URL patterns.""" return self._git_config.get_config().keys() def get_git_data(self, url_pattern: str) -> GitInstanceCfg: + """Return the git config for a specific URL pattern.""" data = self._git_config.get_config().get(url_pattern) if not data: raise KeyError(f"Git config doesn't contain entry for {url_pattern}") @@ -73,4 +81,5 @@ def get_git_data(self, url_pattern: str) -> GitInstanceCfg: raise ValueError(f"Invalid config for {url_pattern}: {e}") def write_git_data(self, pattern: str, git_config: GitInstanceCfg): + """Persist the config for a specific git host.""" self._git_config.update_config({pattern: git_config.model_dump(exclude_none=True)}) diff --git a/src/sc/review/core/ticket.py b/src/sc/review/core/ticket.py index e463393..36c3e47 100644 --- a/src/sc/review/core/ticket.py +++ b/src/sc/review/core/ticket.py @@ -14,7 +14,7 @@ from dataclasses import dataclass @dataclass -class Ticket(): +class Ticket: url: str author: str | None = None assignee: str | None = None @@ -23,4 +23,3 @@ class Ticket(): status: str | None = None target_version: str | None = None title: str | None = None - url: str | None = None diff --git a/src/sc/review/factories/git_factory.py b/src/sc/review/factories/git_factory.py index 7ad97c9..0ff1ff0 100644 --- a/src/sc/review/factories/git_factory.py +++ b/src/sc/review/factories/git_factory.py @@ -22,7 +22,7 @@ class GitFactory: } @classmethod - def names(cls) -> list[str]: + def types(cls) -> list[str]: return list(cls._registry.keys()) @classmethod diff --git a/src/sc/review/factories/ticket_instance_factory.py b/src/sc/review/factories/ticket_instance_factory.py index 1964c30..5b4e64f 100644 --- a/src/sc/review/factories/ticket_instance_factory.py +++ b/src/sc/review/factories/ticket_instance_factory.py @@ -32,31 +32,18 @@ def create( cert: str | None = None ) -> TicketingInstance: if provider == "redmine": - urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) - instance = Redmine( + return RedmineInstance( url, - key=token, - requests={'verify': False} + token=token ) - return RedmineInstance(instance) - elif provider == "jira": - options = {} - if cert: - options["client_cert"] = cert - if auth_type == "token": - instance = JIRA( - server=url, - token_auth=token, - options=options - ) - elif auth_type == "basic": - instance = JIRA( - server=url, - basic_auth=(username, token), - options=options - ) - return JiraInstance(instance) + return JiraInstance( + url, + token=token, + auth_type=auth_type, + username=username, + cert=cert + ) else: raise TicketIdentifierNotFound( f"Provider {provider} doesn't match any ticketing instance!") diff --git a/src/sc/review/git_instances/github_instance.py b/src/sc/review/git_instances/github_instance.py index aa5bdf7..4b0c9c6 100644 --- a/src/sc/review/git_instances/github_instance.py +++ b/src/sc/review/git_instances/github_instance.py @@ -31,13 +31,15 @@ def validate_connection(self) -> bool: url = f"{self.base_url}/user" try: r = requests.get(url, headers=self._headers(), timeout=10) - if r.status_code == 401: - raise ConnectionError("Invalid GitHub token.") - elif r.status_code >= 400: - raise ConnectionError(f"GitHub API error: {r.status_code}") + 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 == 400: + 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 diff --git a/src/sc/review/git_instances/gitlab_instance.py b/src/sc/review/git_instances/gitlab_instance.py index 22036ae..1b8c6ef 100644 --- a/src/sc/review/git_instances/gitlab_instance.py +++ b/src/sc/review/git_instances/gitlab_instance.py @@ -29,13 +29,16 @@ def validate_connection(self) -> bool: url = f"{self.base_url}/api/v4/user" try: r = requests.get(url, headers=self._headers(), timeout=10) - if r.status_code == 401 or r.status_code == 403: - raise ConnectionError("Invalid GitLab token or insufficient permissions.") - elif r.status_code >= 400: - raise ConnectionError(f"GitLab API error: {r.status_code}") + r.raise_for_status() return True except requests.exceptions.Timeout as e: raise ConnectionError("GitLab API request timed out.") from e + except requests.exceptions.HTTPError as e: + status = e.response.status_code + if status == 401 or status == 403: + raise ConnectionError( + "Invalid GitLab token or insufficient permissions.") from e + raise ConnectionError(f"GitLab API error: {status}") from e except requests.exceptions.ConnectionError as e: raise ConnectionError("Network connection to GitLab failed.") from e diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 2f880a3..144bc65 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -21,7 +21,6 @@ from urllib import parse from git import Repo -from rich import print from git_flow_library import GitFlowLibrary from sc_manifest_parser import ScManifest @@ -76,8 +75,6 @@ def run_git_command(self): if self._prompt_yn("Update ticket?"): ticket_comment = self._generate_ticket_comment(comment_data) - if not (ticketing_instance and ticket_id): - ticketing_instance, ticket_id = self._prompt_ticket_selection() ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) def run_repo_command(self): @@ -137,7 +134,7 @@ def _match_branch(self, branch_name: str) -> tuple[str, str]: Returns: tuple[str, str]: (matched_identifier, ticket_number) """ - host_identifiers = self._config.get_ticket_host_ids() + host_identifiers = self._config.get_ticket_host_identifiers() for identifier in host_identifiers: # Matches the identifier, followed by - or _, followed by a number if m := re.search(fr'{identifier}[-_]?(\d+)', branch_name): @@ -264,7 +261,7 @@ def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: logger.info(f"{id} --- {conf.url} --- {conf.description or ''}") input_id = input("> ") - while input_id not in ticket_conf.keys: + while input_id not in ticket_conf.keys(): logger.info("Prefix {input_id} not found in instances.") input_id = input("> ") @@ -286,11 +283,10 @@ def _generate_terminal_comment(self, data: CommentData) -> str: data (CommentData): The data collated from one repo. Returns: - str: Information from one repo to be displayed in the terminal - formatted with markup to be printed with rich. + str: Information from one repo to be displayed in the terminal. """ - def c(colour, text): - return f"[{colour}]{text}[/{colour}]" + def c(code, text): + return f"\033[{code}m{text}\033[0m" header = [ f"Branch: [{data.branch}]", @@ -299,11 +295,11 @@ def c(colour, text): ] if data.review_url: - review_status = f"Review Status: [{c('green', data.review_status)}]" - review_link = f"Review URL: [{c('green', data.review_url)}]" + review_status = f"Review Status: [{c('32', data.review_status)}]" + review_link = f"Review URL: [{c('32', data.review_url)}]" else: - review_status = f"Review Status: [{c('red', data.review_status)}]" - review_link = f"Create Review URL: [{c('yellow', data.create_pr_url)}]" + review_status = f"Review Status: [{c('31', data.review_status)}]" + review_link = f"Create Review URL: [{c('33', data.create_pr_url)}]" review = [review_status, review_link] diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py index 15f76a2..51f6ef2 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. from requests.exceptions import RequestException +from typing import Literal from jira import JIRA from jira.exceptions import JIRAError @@ -38,9 +39,28 @@ class JiraInstance(TicketingInstance): def __init__( self, - instance: JIRA + url: str, + token: str, + auth_type: Literal["token", "basic"] = "token", + username: str | None = None, + cert: str | None = None ): - self._instance = instance + self.url = url + options = {} + if cert: + options["client_cert"] = cert + if auth_type == "token": + self._instance = JIRA( + server=url, + token_auth=token, + options=options + ) + elif auth_type == "basic": + self._instance = JIRA( + server=url, + basic_auth=(username, token), + options=options + ) @property def engine(self) -> str: @@ -87,8 +107,7 @@ def read_ticket(self, ticket_id: str) -> Ticket: versions = getattr(f, "fixVersions", []) target_version = ", ".join(v.name for v in versions) - base_url = self._instance._options["server"] - ticket_url = f'{base_url}/browse/{ticket_id}' + ticket_url = f'{self.url}/browse/{ticket_id}' return Ticket( url=ticket_url, @@ -111,19 +130,6 @@ def add_comment_to_ticket(self, ticket_id: str, comment_message: str): comment = self._convert_from_html(comment_message) return self._instance.add_comment(ticket_id, comment=comment) - def _update_ticket(self, ticket_id: str, **kwargs): - """Updates the ticket with the new value for the fields based on the kwargs - - Args: - ticket_id (str): The id of the ticket to update. Defaults to None. - """ - try: - self._instance.issue(ticket_id).update(**kwargs) - except JIRAError as e: - raise TicketNotFound(e.url) - ticket = self.read_ticket(ticket_id) - return ticket - def _convert_from_html(self, string: str) -> str: string = string.replace('

', '}') diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/redmine_instance.py index 3de50c7..e0d6ef5 100644 --- a/src/sc/review/ticketing_instances/redmine_instance.py +++ b/src/sc/review/ticketing_instances/redmine_instance.py @@ -11,6 +11,7 @@ # 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 urllib3 from redminelib import Redmine from redminelib.resources import Issue @@ -28,9 +29,17 @@ class RedmineInstance(TicketingInstance): def __init__( self, - instance: Redmine + url: str, + token: str, + verify_ssl: bool = False ): - self._instance = instance + self._instance = Redmine( + url, + key=token, + requests={'verify': verify_ssl} + ) + if not verify_ssl: + urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) @property def engine(self) -> str: @@ -102,7 +111,18 @@ def read_ticket(self, ticket_id: str) -> Ticket: target_version=target_version ) - def update_ticket(self, ticket_id: str, **kwargs): + def add_comment_to_ticket(self, ticket_id: str, comment_message: str) -> None: + """Add a comment to a ticket on the redmine instance + + Args: + comment_message (str): The message to add as a comment + ticket_id (str): The ticket number to add the comment to. + """ + ticket = self._update_ticket( + ticket_id, notes=self._convert_html_colours(comment_message)) + return ticket + + def _update_ticket(self, ticket_id: str, **kwargs): """Writes the changed fields from the kwargs, back to the ticket Raises: @@ -125,17 +145,6 @@ def update_ticket(self, ticket_id: str, **kwargs): ticket = self.read_ticket(ticket_id) return ticket - def add_comment_to_ticket(self, ticket_id: str, comment_message: str) -> None: - """Add a comment to a ticket on the redmine instance - - Args: - comment_message (str): The message to add as a comment - ticket_id (str): The ticket number to add the comment to. - """ - ticket = self.update_ticket( - ticket_id, notes=self._convert_html_colours(comment_message)) - return ticket - def _convert_html_colours(self, string: str) -> str: """Convert a html colour tags to redmine formatted colour tags. From 8e1f338347cdec09bdc57b57ee03595cc8950e88 Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Wed, 17 Dec 2025 13:32:08 +0000 Subject: [PATCH 09/12] BWDO-520 Copilot changes --- src/sc/review/factories/__init__.py | 2 +- .../factories/ticket_instance_factory.py | 5 ---- .../review/git_instances/github_instance.py | 19 +++++++++++--- .../review/git_instances/gitlab_instance.py | 25 ++++++++++++++++--- src/sc/review/main.py | 3 ++- src/sc/review/review.py | 8 +++--- 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/sc/review/factories/__init__.py b/src/sc/review/factories/__init__.py index dfd2402..7b6ad7b 100644 --- a/src/sc/review/factories/__init__.py +++ b/src/sc/review/factories/__init__.py @@ -1,2 +1,2 @@ from .git_factory import GitFactory -from . ticket_instance_factory import TicketingInstanceFactory \ No newline at end of file +from .ticket_instance_factory import TicketingInstanceFactory \ No newline at end of file diff --git a/src/sc/review/factories/ticket_instance_factory.py b/src/sc/review/factories/ticket_instance_factory.py index 5b4e64f..f753ee9 100644 --- a/src/sc/review/factories/ticket_instance_factory.py +++ b/src/sc/review/factories/ticket_instance_factory.py @@ -11,15 +11,10 @@ # 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 urllib3 - from sc.review.exceptions import TicketIdentifierNotFound from ..ticketing_instances import JiraInstance, RedmineInstance from ..core.ticketing_instance import TicketingInstance -from jira import JIRA -from redminelib import Redmine - class TicketingInstanceFactory: @classmethod def create( diff --git a/src/sc/review/git_instances/github_instance.py b/src/sc/review/git_instances/github_instance.py index 4b0c9c6..650cd46 100644 --- a/src/sc/review/git_instances/github_instance.py +++ b/src/sc/review/git_instances/github_instance.py @@ -47,9 +47,22 @@ def get_code_review(self, repo: str, source_branch: str) -> CodeReview | None: url = f"{self.base_url}/repos/{repo}/pulls" owner = repo.split("/")[0] params = {"state": "all", "head": f"{owner}:{source_branch}"} - r = requests.get(url, headers=self._headers(), params=params, timeout=10) - r.raise_for_status() - prs = r.json() + + try: + r = requests.get(url, headers=self._headers(), params=params, timeout=10) + r.raise_for_status() + prs = r.json() + 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: + raise RuntimeError("GitHub request failed") from e + if not prs: return None pr = prs[0] diff --git a/src/sc/review/git_instances/gitlab_instance.py b/src/sc/review/git_instances/gitlab_instance.py index 1b8c6ef..9a8d45f 100644 --- a/src/sc/review/git_instances/gitlab_instance.py +++ b/src/sc/review/git_instances/gitlab_instance.py @@ -46,9 +46,21 @@ def get_code_review(self, repo: str, source_branch: str) -> CodeReview: safe_repo = urllib.parse.quote(repo, safe='') url = f"{self.base_url}/api/v4/projects/{safe_repo}/merge_requests" params = {"state": "all", "source_branch": source_branch} - r = requests.get(url, headers=self._headers(), params=params, timeout=10) - r.raise_for_status() - prs = r.json() + try: + r = requests.get(url, headers=self._headers(), params=params, timeout=10) + r.raise_for_status() + prs = r.json() + except requests.Timeout as e: + raise RuntimeError("Gitlab request timed out") from e + except requests.HTTPError as e: + raise RuntimeError( + f"Gitlab API error {r.status_code}: {r.text}" + ) from e + except ValueError as e: # JSON decode error + raise RuntimeError("Invalid JSON from Gitlab API") from e + except requests.RequestException as e: + raise RuntimeError("Gitlab request failed") from e + if not prs: return None pr = prs[0] @@ -63,7 +75,12 @@ def get_code_review(self, repo: str, source_branch: str) -> CodeReview: return CodeReview(url=pr["web_url"], status=status) - def get_create_cr_url(self, repo, source_branch, target_branch = "develop"): + def get_create_cr_url( + self, + repo: str, + source_branch: str, + target_branch: str="develop" + ): params = { "merge_request[source_branch]": source_branch, "merge_request[target_branch]": target_branch, diff --git a/src/sc/review/main.py b/src/sc/review/main.py index 5287af8..f2da1f5 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -49,7 +49,7 @@ def add_git_instance(): print("") if provider == "github": - url = "http://api.github.com" + url = "https://api.github.com" logger.info("Enter a pattern to identify Git from remote url: ") logger.info( "E.G. github.com for all github instances or " @@ -122,6 +122,7 @@ def add_ticketing_instance(): else: project_prefix = None username = None + auth_type = "token" logger.info("Enter the base URL: ") base_url = input("> ") diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 144bc65..1a06e5f 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -48,8 +48,8 @@ class CommentData: class Review: - def __init__(self, top_dir: str): - self.top_dir = top_dir + def __init__(self, top_dir: Path | str): + self.top_dir = Path(top_dir) self._config = ReviewConfig() @@ -101,7 +101,7 @@ def run_repo_command(self): continue proj_repo = Repo(self.top_dir / project.path) - #Don't generate for projects that haven't got an upstream + # Don't generate for projects that haven't got an upstream if not proj_repo.active_branch.tracking_branch(): continue @@ -262,7 +262,7 @@ def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: input_id = input("> ") while input_id not in ticket_conf.keys(): - logger.info("Prefix {input_id} not found in instances.") + logger.info(f"Prefix {input_id} not found in instances.") input_id = input("> ") logger.info("Please enter your ticket number:") From 8737e845718699b4694a96b22266644dbbb1d788 Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Mon, 5 Jan 2026 15:49:20 +0000 Subject: [PATCH 10/12] BWDO-520 changes after review --- src/sc/review/{core => }/code_review.py | 0 src/sc/review/core/__init__.py | 0 src/sc/review/factories/__init__.py | 2 -- .../git_factory.py | 4 ++-- .../{core => git_instances}/git_instance.py | 2 +- .../review/git_instances/github_instance.py | 4 ++-- .../review/git_instances/gitlab_instance.py | 4 ++-- src/sc/review/main.py | 6 +++--- src/sc/review/review.py | 21 ++++++++++--------- src/sc/review/{core => }/review_config.py | 0 src/sc/review/{core => }/ticket.py | 0 .../ticketing_instances/jira_instance.py | 15 +++++++------ .../ticketing_instances/redmine_instance.py | 4 ++-- .../ticket_instance_factory.py | 4 ++-- .../ticketing_instance.py | 2 +- 15 files changed, 33 insertions(+), 35 deletions(-) rename src/sc/review/{core => }/code_review.py (100%) delete mode 100644 src/sc/review/core/__init__.py delete mode 100644 src/sc/review/factories/__init__.py rename src/sc/review/{factories => git_instances}/git_factory.py (91%) rename src/sc/review/{core => git_instances}/git_instance.py (98%) rename src/sc/review/{core => }/review_config.py (100%) rename src/sc/review/{core => }/ticket.py (100%) rename src/sc/review/{factories => ticketing_instances}/ticket_instance_factory.py (92%) rename src/sc/review/{core => ticketing_instances}/ticketing_instance.py (98%) diff --git a/src/sc/review/core/code_review.py b/src/sc/review/code_review.py similarity index 100% rename from src/sc/review/core/code_review.py rename to src/sc/review/code_review.py diff --git a/src/sc/review/core/__init__.py b/src/sc/review/core/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/src/sc/review/factories/__init__.py b/src/sc/review/factories/__init__.py deleted file mode 100644 index 7b6ad7b..0000000 --- a/src/sc/review/factories/__init__.py +++ /dev/null @@ -1,2 +0,0 @@ -from .git_factory import GitFactory -from .ticket_instance_factory import TicketingInstanceFactory \ No newline at end of file diff --git a/src/sc/review/factories/git_factory.py b/src/sc/review/git_instances/git_factory.py similarity index 91% rename from src/sc/review/factories/git_factory.py rename to src/sc/review/git_instances/git_factory.py index 0ff1ff0..38b8b42 100644 --- a/src/sc/review/factories/git_factory.py +++ b/src/sc/review/git_instances/git_factory.py @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from ..git_instances import GithubInstance, GitlabInstance -from ..core.git_instance import GitInstance +from . import GithubInstance, GitlabInstance +from .git_instance import GitInstance class GitFactory: _registry = { diff --git a/src/sc/review/core/git_instance.py b/src/sc/review/git_instances/git_instance.py similarity index 98% rename from src/sc/review/core/git_instance.py rename to src/sc/review/git_instances/git_instance.py index 4230f32..0d39543 100644 --- a/src/sc/review/core/git_instance.py +++ b/src/sc/review/git_instances/git_instance.py @@ -14,7 +14,7 @@ from abc import ABC, abstractmethod -from .code_review import CodeReview +from ..code_review import CodeReview class GitInstance(ABC): @abstractmethod diff --git a/src/sc/review/git_instances/github_instance.py b/src/sc/review/git_instances/github_instance.py index 650cd46..3f178b0 100644 --- a/src/sc/review/git_instances/github_instance.py +++ b/src/sc/review/git_instances/github_instance.py @@ -14,8 +14,8 @@ import requests -from ..core.code_review import CRStatus, CodeReview -from ..core.git_instance import GitInstance +from ..code_review import CRStatus, CodeReview +from .git_instance import GitInstance class GithubInstance(GitInstance): def __init__(self, token: str, base_url: str | None): diff --git a/src/sc/review/git_instances/gitlab_instance.py b/src/sc/review/git_instances/gitlab_instance.py index 9a8d45f..76efb68 100644 --- a/src/sc/review/git_instances/gitlab_instance.py +++ b/src/sc/review/git_instances/gitlab_instance.py @@ -15,8 +15,8 @@ import requests import urllib.parse -from ..core.code_review import CodeReview, CRStatus -from ..core.git_instance import GitInstance +from ..code_review import CodeReview, CRStatus +from .git_instance import GitInstance class GitlabInstance(GitInstance): def __init__(self, token: str, base_url: str): diff --git a/src/sc/review/main.py b/src/sc/review/main.py index f2da1f5..1a4ad53 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -21,9 +21,9 @@ from repo_library import RepoLibrary from .exceptions import ReviewException from .review import Review -from .core.review_config import ReviewConfig, TicketHostCfg, GitInstanceCfg -from .factories.ticket_instance_factory import TicketingInstanceFactory -from .factories.git_factory import GitFactory +from .review_config import ReviewConfig, TicketHostCfg, GitInstanceCfg +from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory +from .git_instances.git_factory import GitFactory logger = logging.getLogger(__name__) diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 1a06e5f..5285aa4 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -25,11 +25,11 @@ from git_flow_library import GitFlowLibrary from sc_manifest_parser import ScManifest from .exceptions import RemoteUrlNotFound, TicketIdentifierNotFound -from .core.review_config import ReviewConfig, TicketHostCfg -from .factories.ticket_instance_factory import TicketingInstanceFactory -from .core.ticketing_instance import TicketingInstance -from .factories.git_factory import GitFactory -from .core.git_instance import GitInstance +from .review_config import ReviewConfig, TicketHostCfg +from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory +from .ticketing_instances.ticketing_instance import TicketingInstance +from .git_instances.git_factory import GitFactory +from .git_instances.git_instance import GitInstance logger = logging.getLogger(__name__) @@ -61,7 +61,8 @@ def run_git_command(self): logger.warning(e) identifier, ticket_num = self._prompt_ticket_selection() - ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) + ticketing_cfg = self._config.get_ticket_host_data(identifier) + ticketing_instance = self._create_ticketing_instance(identifier) ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) @@ -86,7 +87,8 @@ def run_repo_command(self): logger.warning(e) identifier, ticket_num = self._prompt_ticket_selection() - ticketing_instance, ticketing_cfg = self._load_ticketing(identifier) + ticketing_cfg = self._config.get_ticket_host_data(identifier) + ticketing_instance = self._create_ticketing_instance(identifier) ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) @@ -231,8 +233,7 @@ def _create_comment_data(self, repo: Repo, git_instance: GitInstance) -> Comment commit_message=commit.message.strip() ) - def _load_ticketing(self, identifier: str) -> tuple[TicketingInstance, TicketHostCfg]: - cfg = self._config.get_ticket_host_data(identifier) + def _create_ticketing_instance(self, cfg: TicketHostCfg) -> TicketingInstance: inst = TicketingInstanceFactory.create( provider=cfg.provider, url=cfg.url, @@ -241,7 +242,7 @@ def _load_ticketing(self, identifier: str) -> tuple[TicketingInstance, TicketHos username=cfg.username, cert=cfg.cert ) - return inst, cfg + return inst def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: """Prompt the user to select a ticketing instance and enter a ticket number. diff --git a/src/sc/review/core/review_config.py b/src/sc/review/review_config.py similarity index 100% rename from src/sc/review/core/review_config.py rename to src/sc/review/review_config.py diff --git a/src/sc/review/core/ticket.py b/src/sc/review/ticket.py similarity index 100% rename from src/sc/review/core/ticket.py rename to src/sc/review/ticket.py diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/jira_instance.py index 51f6ef2..2050c48 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/jira_instance.py @@ -18,20 +18,19 @@ from jira.exceptions import JIRAError from ..exceptions import PermissionsError, TicketNotFound -from ..core.ticket import Ticket -from ..core.ticketing_instance import TicketingInstance +from ..ticket import Ticket +from .ticketing_instance import TicketingInstance class JiraInstance(TicketingInstance): """A class to handle operations on Jira tickets. Args: url (str): URL of Jira instance. - password (str): Password to authenticate with. - cert (str | None): Path to a cert file. - - Common KWargs: - verify (bool): Whether to use ssl verification. Default True - extra_setup_options (dict): extra setup options for the jira instance + token (str): Token to authenticate with. + auth_type (str): Either token for token auth or basic for basic auth. Basic + auth takes an email and password, token takes a token. + username (str): The username if using basic auth. + cert (str): The cert. Raises: TicketingInstanceUnreachable: If the ticketing instance cannot be connected to. diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/redmine_instance.py index e0d6ef5..237224d 100644 --- a/src/sc/review/ticketing_instances/redmine_instance.py +++ b/src/sc/review/ticketing_instances/redmine_instance.py @@ -19,8 +19,8 @@ from requests.exceptions import RequestException, SSLError from ..exceptions import TicketNotFound, TicketingInstanceUnreachable, PermissionsError -from ..core.ticketing_instance import TicketingInstance -from ..core.ticket import Ticket +from .ticketing_instance import TicketingInstance +from ..ticket import Ticket class RedmineInstance(TicketingInstance): """ diff --git a/src/sc/review/factories/ticket_instance_factory.py b/src/sc/review/ticketing_instances/ticket_instance_factory.py similarity index 92% rename from src/sc/review/factories/ticket_instance_factory.py rename to src/sc/review/ticketing_instances/ticket_instance_factory.py index f753ee9..67a6463 100644 --- a/src/sc/review/factories/ticket_instance_factory.py +++ b/src/sc/review/ticketing_instances/ticket_instance_factory.py @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. from sc.review.exceptions import TicketIdentifierNotFound -from ..ticketing_instances import JiraInstance, RedmineInstance -from ..core.ticketing_instance import TicketingInstance +from . import JiraInstance, RedmineInstance +from .ticketing_instance import TicketingInstance class TicketingInstanceFactory: @classmethod diff --git a/src/sc/review/core/ticketing_instance.py b/src/sc/review/ticketing_instances/ticketing_instance.py similarity index 98% rename from src/sc/review/core/ticketing_instance.py rename to src/sc/review/ticketing_instances/ticketing_instance.py index 62cdc05..0f3eff2 100644 --- a/src/sc/review/core/ticketing_instance.py +++ b/src/sc/review/ticketing_instances/ticketing_instance.py @@ -14,7 +14,7 @@ from abc import ABC, abstractmethod -from .ticket import Ticket +from ..ticket import Ticket class TicketingInstance(ABC): """ From b6a9b80fe996c2bf8de2d6862ac5798064e35cf1 Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Tue, 6 Jan 2026 13:43:47 +0000 Subject: [PATCH 11/12] BWDO-520 changes after review --- src/sc/review/git_instances/__init__.py | 4 +- src/sc/review/git_instances/git_factory.py | 2 +- .../git_instances/instances/__init__.py | 2 + .../{ => instances}/github_instance.py | 6 +- .../{ => instances}/gitlab_instance.py | 4 +- src/sc/review/main.py | 23 ++++--- src/sc/review/review.py | 21 +++++-- src/sc/review/ticketing_instances/__init__.py | 4 +- .../ticketing_instances/instances/__init__.py | 2 + .../{ => instances}/jira_instance.py | 56 ++++++++--------- .../{ => instances}/redmine_instance.py | 61 +++++++++++-------- .../ticket_instance_factory.py | 2 +- .../ticketing_instances/ticketing_instance.py | 12 ---- 13 files changed, 102 insertions(+), 97 deletions(-) create mode 100644 src/sc/review/git_instances/instances/__init__.py rename src/sc/review/git_instances/{ => instances}/github_instance.py (96%) rename src/sc/review/git_instances/{ => instances}/gitlab_instance.py (97%) create mode 100644 src/sc/review/ticketing_instances/instances/__init__.py rename src/sc/review/ticketing_instances/{ => instances}/jira_instance.py (75%) rename src/sc/review/ticketing_instances/{ => instances}/redmine_instance.py (89%) diff --git a/src/sc/review/git_instances/__init__.py b/src/sc/review/git_instances/__init__.py index e8c8830..3d289e5 100644 --- a/src/sc/review/git_instances/__init__.py +++ b/src/sc/review/git_instances/__init__.py @@ -1,2 +1,2 @@ -from .github_instance import GithubInstance -from .gitlab_instance import GitlabInstance \ No newline at end of file +from .git_factory import GitFactory +from .git_instance import GitInstance \ No newline at end of file diff --git a/src/sc/review/git_instances/git_factory.py b/src/sc/review/git_instances/git_factory.py index 38b8b42..ab7cbbd 100644 --- a/src/sc/review/git_instances/git_factory.py +++ b/src/sc/review/git_instances/git_factory.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from . import GithubInstance, GitlabInstance +from .instances import GithubInstance, GitlabInstance from .git_instance import GitInstance class GitFactory: diff --git a/src/sc/review/git_instances/instances/__init__.py b/src/sc/review/git_instances/instances/__init__.py new file mode 100644 index 0000000..e8c8830 --- /dev/null +++ b/src/sc/review/git_instances/instances/__init__.py @@ -0,0 +1,2 @@ +from .github_instance import GithubInstance +from .gitlab_instance import GitlabInstance \ No newline at end of file diff --git a/src/sc/review/git_instances/github_instance.py b/src/sc/review/git_instances/instances/github_instance.py similarity index 96% rename from src/sc/review/git_instances/github_instance.py rename to src/sc/review/git_instances/instances/github_instance.py index 3f178b0..f0d732c 100644 --- a/src/sc/review/git_instances/github_instance.py +++ b/src/sc/review/git_instances/instances/github_instance.py @@ -14,8 +14,8 @@ import requests -from ..code_review import CRStatus, CodeReview -from .git_instance import GitInstance +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): @@ -82,4 +82,4 @@ def get_create_cr_url( source_branch: str, target_branch: str = "develop" ) -> str: - return f"https://github.com/{repo}/compare/{target_branch}...{source_branch}" \ No newline at end of file + return f"https://github.com/{repo}/compare/{target_branch}...{source_branch}" diff --git a/src/sc/review/git_instances/gitlab_instance.py b/src/sc/review/git_instances/instances/gitlab_instance.py similarity index 97% rename from src/sc/review/git_instances/gitlab_instance.py rename to src/sc/review/git_instances/instances/gitlab_instance.py index 76efb68..c8e8734 100644 --- a/src/sc/review/git_instances/gitlab_instance.py +++ b/src/sc/review/git_instances/instances/gitlab_instance.py @@ -15,8 +15,8 @@ import requests import urllib.parse -from ..code_review import CodeReview, CRStatus -from .git_instance import GitInstance +from sc.review.code_review import CodeReview, CRStatus +from ..git_instance import GitInstance class GitlabInstance(GitInstance): def __init__(self, token: str, base_url: str): diff --git a/src/sc/review/main.py b/src/sc/review/main.py index 1a4ad53..33f3d8b 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -22,8 +22,8 @@ from .exceptions import ReviewException from .review import Review from .review_config import ReviewConfig, TicketHostCfg, GitInstanceCfg -from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory -from .git_instances.git_factory import GitFactory +from .ticketing_instances import TicketingInstanceFactory +from .git_instances import GitFactory logger = logging.getLogger(__name__) @@ -36,7 +36,7 @@ def review(): else: logger.error("Not in a repo project or git repository!") sys.exit(1) - except ReviewException as e: + except (ReviewException, ConnectionError) as e: logger.error(e) sys.exit(1) @@ -101,6 +101,7 @@ def add_ticketing_instance(): branch_prefix = input("> ") print("") + username = None if provider == "jira": project_prefix = f"{branch_prefix}-" @@ -121,7 +122,6 @@ def add_ticketing_instance(): else: project_prefix = None - username = None auth_type = "token" logger.info("Enter the base URL: ") @@ -132,15 +132,14 @@ def add_ticketing_instance(): api_token = getpass.getpass("> ") print("") - instance = TicketingInstanceFactory.create( - provider=provider, - url=base_url, - token=api_token, - auth_type=auth_type, - username=username - ) try: - instance.validate_connection() + TicketingInstanceFactory.create( + provider=provider, + url=base_url, + token=api_token, + auth_type=auth_type, + username=username + ) except ConnectionError as e: logger.error(f"Failed to connect! {e}") sys.exit(1) diff --git a/src/sc/review/review.py b/src/sc/review/review.py index 5285aa4..bcd3e44 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -26,10 +26,8 @@ from sc_manifest_parser import ScManifest from .exceptions import RemoteUrlNotFound, TicketIdentifierNotFound from .review_config import ReviewConfig, TicketHostCfg -from .ticketing_instances.ticket_instance_factory import TicketingInstanceFactory -from .ticketing_instances.ticketing_instance import TicketingInstance -from .git_instances.git_factory import GitFactory -from .git_instances.git_instance import GitInstance +from .ticketing_instances import TicketingInstance, TicketingInstanceFactory +from .git_instances import GitFactory, GitInstance logger = logging.getLogger(__name__) @@ -62,7 +60,7 @@ def run_git_command(self): identifier, ticket_num = self._prompt_ticket_selection() ticketing_cfg = self._config.get_ticket_host_data(identifier) - ticketing_instance = self._create_ticketing_instance(identifier) + ticketing_instance = self._create_ticketing_instance(ticketing_cfg) ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) @@ -88,7 +86,7 @@ def run_repo_command(self): identifier, ticket_num = self._prompt_ticket_selection() ticketing_cfg = self._config.get_ticket_host_data(identifier) - ticketing_instance = self._create_ticketing_instance(identifier) + ticketing_instance = self._create_ticketing_instance(ticketing_cfg) ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) @@ -234,6 +232,17 @@ def _create_comment_data(self, repo: Repo, git_instance: GitInstance) -> Comment ) def _create_ticketing_instance(self, cfg: TicketHostCfg) -> TicketingInstance: + """Create a ticketing instance. + + Args: + cfg (TicketHostCfg): Config describing a ticketing instance. + + Raises: + ConnectionError: Failed to connect to ticketing instance. + + Returns: + TicketingInstance: A ticketing instance class. + """ inst = TicketingInstanceFactory.create( provider=cfg.provider, url=cfg.url, diff --git a/src/sc/review/ticketing_instances/__init__.py b/src/sc/review/ticketing_instances/__init__.py index 33ad7ae..f5a678f 100644 --- a/src/sc/review/ticketing_instances/__init__.py +++ b/src/sc/review/ticketing_instances/__init__.py @@ -1,2 +1,2 @@ -from .jira_instance import JiraInstance -from .redmine_instance import RedmineInstance \ No newline at end of file +from .ticketing_instance import TicketingInstance +from .ticket_instance_factory import TicketingInstanceFactory \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/instances/__init__.py b/src/sc/review/ticketing_instances/instances/__init__.py new file mode 100644 index 0000000..33ad7ae --- /dev/null +++ b/src/sc/review/ticketing_instances/instances/__init__.py @@ -0,0 +1,2 @@ +from .jira_instance import JiraInstance +from .redmine_instance import RedmineInstance \ No newline at end of file diff --git a/src/sc/review/ticketing_instances/jira_instance.py b/src/sc/review/ticketing_instances/instances/jira_instance.py similarity index 75% rename from src/sc/review/ticketing_instances/jira_instance.py rename to src/sc/review/ticketing_instances/instances/jira_instance.py index 2050c48..7ef18fd 100644 --- a/src/sc/review/ticketing_instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/instances/jira_instance.py @@ -17,9 +17,9 @@ from jira import JIRA from jira.exceptions import JIRAError -from ..exceptions import PermissionsError, TicketNotFound -from ..ticket import Ticket -from .ticketing_instance import TicketingInstance +from sc.review.exceptions import PermissionsError, TicketNotFound +from sc.review.ticket import Ticket +from .. import TicketingInstance class JiraInstance(TicketingInstance): """A class to handle operations on Jira tickets. @@ -48,37 +48,35 @@ def __init__( options = {} if cert: options["client_cert"] = cert - if auth_type == "token": - self._instance = JIRA( - server=url, - token_auth=token, - options=options - ) - elif auth_type == "basic": - self._instance = JIRA( - server=url, - basic_auth=(username, token), - options=options - ) + try: + if auth_type == "token": + self._instance = JIRA( + server=url, + token_auth=token, + options=options + ) + elif auth_type == "basic": + self._instance = JIRA( + server=url, + basic_auth=(username, token), + options=options + ) + except JIRAError as e: + if e.status_code in (401, 403): + raise ConnectionError( + f"Invalid Jira credentials or insufficient permissions " + f"for instance {url}. {e.text}" + ) from e + raise ConnectionAbortedError( + f"Jira API error for instance {url}: HTTP {e.status_code}") from e + + except RequestException as e: + raise ConnectionError(f"Unable to reach Jira server {url}.") from e @property def engine(self) -> str: return 'jira' - def validate_connection(self) -> bool: - """Check that the Jira instance and credentials are valid.""" - try: - self._instance.myself() - return True - - except JIRAError as e: - if e.status_code in (401, 403): - raise ConnectionError("Invalid Jira credentials or insufficient permissions.") from e - raise ConnectionAbortedError(f"Jira API error: HTTP {e.status_code}") from e - - except RequestException as e: - raise ConnectionError("Unable to reach Jira server.") from e - def read_ticket(self, ticket_id: str) -> Ticket: """Reads the contents of the ticket and puts the dictionary in to contents diff --git a/src/sc/review/ticketing_instances/redmine_instance.py b/src/sc/review/ticketing_instances/instances/redmine_instance.py similarity index 89% rename from src/sc/review/ticketing_instances/redmine_instance.py rename to src/sc/review/ticketing_instances/instances/redmine_instance.py index 237224d..ea34dd1 100644 --- a/src/sc/review/ticketing_instances/redmine_instance.py +++ b/src/sc/review/ticketing_instances/instances/redmine_instance.py @@ -18,9 +18,9 @@ from redminelib.exceptions import BaseRedmineError, ForbiddenError, ResourceNotFoundError, AuthError from requests.exceptions import RequestException, SSLError -from ..exceptions import TicketNotFound, TicketingInstanceUnreachable, PermissionsError -from .ticketing_instance import TicketingInstance -from ..ticket import Ticket +from sc.review.exceptions import PermissionsError, TicketingInstanceUnreachable, TicketNotFound +from sc.review.ticket import Ticket +from .. import TicketingInstance class RedmineInstance(TicketingInstance): """ @@ -33,40 +33,20 @@ def __init__( token: str, verify_ssl: bool = False ): + if not verify_ssl: + urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) + self._instance = Redmine( url, key=token, requests={'verify': verify_ssl} ) - if not verify_ssl: - urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) + self._validate_connection() @property def engine(self) -> str: return 'redmine' - def validate_connection(self) -> bool: - """Check if the Redmine instance and API key are valid. - - Raises: - ConnectionError: If the connection is invalid. - - Returns: - bool: True if connection is valid. - """ - try: - self._instance.auth() - return True - - except (AuthError, ForbiddenError) as e: - raise ConnectionError("Invalid Redmine API key or insufficient permssions.") - - except BaseRedmineError as e: - raise ConnectionError(f"Redmine API error: {e}") - - except RequestException as e: - raise ConnectionError("Failed to reach Redmine server.") from e - def read_ticket(self, ticket_id: str) -> Ticket: """Read the information from a ticket and put it's contents in this object contents dict @@ -145,6 +125,33 @@ def _update_ticket(self, ticket_id: str, **kwargs): ticket = self.read_ticket(ticket_id) return ticket + def _validate_connection(self) -> bool: + """Check if the Redmine instance and API key are valid. + + Raises: + ConnectionError: If the connection is invalid. + + Returns: + bool: True if connection is valid. + """ + try: + self._instance.auth() + return True + + except (AuthError, ForbiddenError) as e: + raise ConnectionError( + "Invalid Redmine API key or insufficient permssions for " + f"{self._instance.url}." + ) from e + + except BaseRedmineError as e: + raise ConnectionError( + f"Redmine API error at {self._instance.url}: {e}") from e + + except RequestException as e: + raise ConnectionError( + f"Failed to reach Redmine server at {self._instance.url}") from e + def _convert_html_colours(self, string: str) -> str: """Convert a html colour tags to redmine formatted colour tags. diff --git a/src/sc/review/ticketing_instances/ticket_instance_factory.py b/src/sc/review/ticketing_instances/ticket_instance_factory.py index 67a6463..7ff1e01 100644 --- a/src/sc/review/ticketing_instances/ticket_instance_factory.py +++ b/src/sc/review/ticketing_instances/ticket_instance_factory.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. from sc.review.exceptions import TicketIdentifierNotFound -from . import JiraInstance, RedmineInstance +from .instances import JiraInstance, RedmineInstance from .ticketing_instance import TicketingInstance class TicketingInstanceFactory: diff --git a/src/sc/review/ticketing_instances/ticketing_instance.py b/src/sc/review/ticketing_instances/ticketing_instance.py index 0f3eff2..25c715d 100644 --- a/src/sc/review/ticketing_instances/ticketing_instance.py +++ b/src/sc/review/ticketing_instances/ticketing_instance.py @@ -26,18 +26,6 @@ class TicketingInstance(ABC): def engine(self) -> str: pass - @abstractmethod - def validate_connection(self) -> bool: - """Abstract Method: - Validate connection to the ticketing instance. - - Raises: - ConnectionError: If the connection is unsuccessful. - - Returns: - bool: True if connection is successful. - """ - @abstractmethod def read_ticket(self, ticket_id: str) -> Ticket: """Abstract Method: From be138b2f4464cdc93ae02e4ca4c3483f837c301b Mon Sep 17 00:00:00 2001 From: Benjamin Milan Date: Tue, 6 Jan 2026 14:17:07 +0000 Subject: [PATCH 12/12] BWDO-520 Copilot changes --- src/sc/review/exceptions.py | 10 +++- src/sc/review/git_instances/git_instance.py | 7 ++- .../instances/github_instance.py | 18 ++++++-- .../instances/gitlab_instance.py | 46 +++++++++++++------ src/sc/review/main.py | 4 +- src/sc/review/review.py | 19 ++++---- src/sc/review/review_config.py | 9 ++-- .../instances/jira_instance.py | 8 ++-- .../instances/redmine_instance.py | 10 ++-- .../ticket_instance_factory.py | 26 ++++++++++- .../ticketing_instances/ticketing_instance.py | 3 +- 11 files changed, 110 insertions(+), 50 deletions(-) diff --git a/src/sc/review/exceptions.py b/src/sc/review/exceptions.py index c05f612..0790c67 100644 --- a/src/sc/review/exceptions.py +++ b/src/sc/review/exceptions.py @@ -48,11 +48,17 @@ def __init__(self, resource: str, resolution_message: str = ''): f'You do not have permission to access {resource}\n{resolution_message}' ) -class TicketIdentifierNotFound(ReviewException): +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(ReviewException): +class RemoteUrlNotFound(ConfigError): """Raised when remote url matches no patterns in the config. """ + pass diff --git a/src/sc/review/git_instances/git_instance.py b/src/sc/review/git_instances/git_instance.py index 0d39543..a30551c 100644 --- a/src/sc/review/git_instances/git_instance.py +++ b/src/sc/review/git_instances/git_instance.py @@ -17,9 +17,8 @@ from ..code_review import CodeReview class GitInstance(ABC): - @abstractmethod def __init__(self, token: str, base_url: str | None): - self.token = token + self._token = token self.base_url = base_url.rstrip("/") if base_url else None @abstractmethod @@ -28,7 +27,7 @@ def validate_connection(self) -> bool: Validates if connection to the git instance is successful. Raises: - ConnectionError: If the connection is unsuccesful. + ConnectionError: If the connection is unsuccessful. Returns: bool: True if connection is successful. @@ -37,7 +36,7 @@ def validate_connection(self) -> bool: @abstractmethod def get_code_review(self, repo: str, source_branch: str) -> CodeReview: - """Get information about about a branches code review. + """Get information about a branches code review. Args: repo (str): An identifier of the repo in the instance e.g "org/repo". diff --git a/src/sc/review/git_instances/instances/github_instance.py b/src/sc/review/git_instances/instances/github_instance.py index f0d732c..d909770 100644 --- a/src/sc/review/git_instances/instances/github_instance.py +++ b/src/sc/review/git_instances/instances/github_instance.py @@ -24,7 +24,7 @@ def __init__(self, token: str, base_url: str | None): def _headers(self) -> dict: return { "Accept": "application/vnd.github.v3+json", - "Authorization": f"Bearer {self.token}" + "Authorization": f"Bearer {self._token}" } def validate_connection(self) -> bool: @@ -37,13 +37,25 @@ def validate_connection(self) -> bool: raise ConnectionError("GitHub API request timed out.") from e except requests.exceptions.HTTPError as e: status = e.response.status_code - if status == 400: + 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] params = {"state": "all", "head": f"{owner}:{source_branch}"} @@ -56,7 +68,7 @@ def get_code_review(self, repo: str, source_branch: str) -> CodeReview | None: raise RuntimeError("GitHub request timed out") from e except requests.HTTPError as e: raise RuntimeError( - f"GitHub API error {r.status_code}: {r.text}" + f"GitHub API error {e.response.status_code}: {e.response.text}" ) from e except ValueError as e: # JSON decode error raise RuntimeError("Invalid JSON from GitHub API") from e diff --git a/src/sc/review/git_instances/instances/gitlab_instance.py b/src/sc/review/git_instances/instances/gitlab_instance.py index c8e8734..c414a9f 100644 --- a/src/sc/review/git_instances/instances/gitlab_instance.py +++ b/src/sc/review/git_instances/instances/gitlab_instance.py @@ -23,43 +23,61 @@ def __init__(self, token: str, base_url: str): super().__init__(token, base_url) def _headers(self) -> dict[str, str]: - return {"Private-Token": self.token} + return {"Private-Token": self._token} def validate_connection(self) -> bool: url = f"{self.base_url}/api/v4/user" try: - r = requests.get(url, headers=self._headers(), timeout=10) + r = requests.get(url, headers=self._headers(), timeout=10, verify=False) r.raise_for_status() return True - except requests.exceptions.Timeout as e: - raise ConnectionError("GitLab API request timed out.") from e - except requests.exceptions.HTTPError as e: + except requests.Timeout as e: + raise ConnectionError( + f"GitLab API request timed out for {self.base_url}") from e + except requests.HTTPError as e: status = e.response.status_code if status == 401 or status == 403: raise ConnectionError( - "Invalid GitLab token or insufficient permissions.") from e - raise ConnectionError(f"GitLab API error: {status}") from e - except requests.exceptions.ConnectionError as e: - raise ConnectionError("Network connection to GitLab failed.") from e + "Invalid GitLab token or insufficient permissions for " + f"{self.base_url}" + ) from e + raise ConnectionError( + f"GitLab API error for {self.base_url}: {status}") from e + except requests.ConnectionError as e: + raise ConnectionError( + f"Network connection to GitLab failed for {self.base_url}") from e def get_code_review(self, repo: str, source_branch: str) -> CodeReview: + """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. + """ safe_repo = urllib.parse.quote(repo, safe='') url = f"{self.base_url}/api/v4/projects/{safe_repo}/merge_requests" params = {"state": "all", "source_branch": source_branch} try: - r = requests.get(url, headers=self._headers(), params=params, timeout=10) + r = requests.get( + url, headers=self._headers(), params=params, timeout=10, verify=False) r.raise_for_status() prs = r.json() except requests.Timeout as e: - raise RuntimeError("Gitlab request timed out") from e + raise RuntimeError(f"Gitlab request timed out for {self.base_url}") from e except requests.HTTPError as e: raise RuntimeError( - f"Gitlab API error {r.status_code}: {r.text}" + f"Gitlab API error {e.response.status_code}: {e.response.text}" ) from e except ValueError as e: # JSON decode error - raise RuntimeError("Invalid JSON from Gitlab API") from e + raise RuntimeError(f"Invalid JSON from Gitlab API for {self.base_url}") from e except requests.RequestException as e: - raise RuntimeError("Gitlab request failed") from e + raise RuntimeError(f"Gitlab request failed for {self.base_url}") from e if not prs: return None diff --git a/src/sc/review/main.py b/src/sc/review/main.py index 33f3d8b..9110798 100644 --- a/src/sc/review/main.py +++ b/src/sc/review/main.py @@ -65,10 +65,10 @@ def add_git_instance(): pattern = url.replace("https://", "").replace("http://", "") else: logger.error("Provider matches none in the list!") - + sys.exit(1) logger.info("Enter your api token: ") - api_key = input("> ") + api_key = getpass.getpass("> ") print("") instance = GitFactory.create(provider, api_key, url) diff --git a/src/sc/review/review.py b/src/sc/review/review.py index bcd3e44..35f6779 100644 --- a/src/sc/review/review.py +++ b/src/sc/review/review.py @@ -52,9 +52,9 @@ def __init__(self, top_dir: Path | str): self._config = ReviewConfig() def run_git_command(self): - branch_name = Repo(self.top_dir).active_branch.name + repo = Repo(self.top_dir) try: - identifier, ticket_num = self._match_branch(branch_name) + identifier, ticket_num = self._match_branch(repo.active_branch.name) except TicketIdentifierNotFound as e: logger.warning(e) identifier, ticket_num = self._prompt_ticket_selection() @@ -65,18 +65,20 @@ def run_git_command(self): ticket_id = f"{ticketing_cfg.project_prefix or ''}{ticket_num}" ticket = ticketing_instance.read_ticket(ticket_id) - git_instance = self._create_git_instance(Repo(self.top_dir).remote().url) - comment_data = self._create_comment_data(Repo(self.top_dir), git_instance) + git_instance = self._create_git_instance(repo.remote().url) + comment_data = self._create_comment_data(repo, git_instance) logger.info(f"Ticket URL: [{ticket.url if ticket else 'None'}]") - logger.info("Ticket info: ") + logger.info("Ticket info: \n") print(self._generate_terminal_comment(comment_data)) + print() if self._prompt_yn("Update ticket?"): ticket_comment = self._generate_ticket_comment(comment_data) ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) def run_repo_command(self): + logger.info("Show check ins across all repos. Note branch must be PUSHED.\n") manifest_repo = Repo(self.top_dir / '.repo' / 'manifests') try: @@ -92,7 +94,7 @@ def run_repo_command(self): ticket = ticketing_instance.read_ticket(ticket_id) logger.info(f"Ticket URL: [{ticket.url if ticket else ''}]") - logger.info("Ticket info: ") + logger.info("Ticket info: \n") manifest = ScManifest.from_repo_root(self.top_dir / '.repo') comments = [] @@ -116,8 +118,9 @@ def run_repo_command(self): comments.append(comment_data) print(self._generate_combined_terminal_comment(comments)) + print() - if self._prompt_yn("Update ticket?"): + if self._prompt_yn("Update tickets?"): ticket_comment = self._generate_combined_ticket_comment(comments) ticketing_instance.add_comment_to_ticket(ticket_id, ticket_comment) @@ -253,7 +256,7 @@ def _create_ticketing_instance(self, cfg: TicketHostCfg) -> TicketingInstance: ) return inst - def _prompt_ticket_selection(self) -> tuple[TicketingInstance, str]: + def _prompt_ticket_selection(self) -> tuple[str, str]: """Prompt the user to select a ticketing instance and enter a ticket number. Raises: diff --git a/src/sc/review/review_config.py b/src/sc/review/review_config.py index 7be328d..1ca3826 100644 --- a/src/sc/review/review_config.py +++ b/src/sc/review/review_config.py @@ -16,6 +16,7 @@ from pydantic import BaseModel, ConfigDict, ValidationError +from .exceptions import ConfigError from sc.config_manager import ConfigManager class TicketHostCfg(BaseModel): @@ -54,12 +55,12 @@ def get_ticket_host_data(self, identifier: str) -> TicketHostCfg: """Return the ticketing config for a specific identifier.""" data = self._ticket_config.get_config().get(identifier) if not data: - raise KeyError( + raise ConfigError( f"Ticket instance config doesn't contain entry for {identifier}") try: return TicketHostCfg(**data) except ValidationError as e: - raise ValueError(f"Invalid config for {identifier}: {e}") + raise ConfigError(f"Invalid config for ticketing instance {identifier}: {e}") def write_ticketing_data(self, branch_prefix: str, ticket_data: TicketHostCfg): """Persist ticketing config for a branch prefix.""" @@ -74,11 +75,11 @@ def get_git_data(self, url_pattern: str) -> GitInstanceCfg: """Return the git config for a specific URL pattern.""" data = self._git_config.get_config().get(url_pattern) if not data: - raise KeyError(f"Git config doesn't contain entry for {url_pattern}") + raise ConfigError(f"Git config doesn't contain entry for {url_pattern}") try: return GitInstanceCfg(**data) except ValidationError as e: - raise ValueError(f"Invalid config for {url_pattern}: {e}") + raise ConfigError(f"Invalid config for git instance {url_pattern}: {e}") def write_git_data(self, pattern: str, git_config: GitInstanceCfg): """Persist the config for a specific git host.""" diff --git a/src/sc/review/ticketing_instances/instances/jira_instance.py b/src/sc/review/ticketing_instances/instances/jira_instance.py index 7ef18fd..cd00fd1 100644 --- a/src/sc/review/ticketing_instances/instances/jira_instance.py +++ b/src/sc/review/ticketing_instances/instances/jira_instance.py @@ -13,6 +13,7 @@ # limitations under the License. from requests.exceptions import RequestException from typing import Literal +import urllib3 from jira import JIRA from jira.exceptions import JIRAError @@ -44,6 +45,7 @@ def __init__( username: str | None = None, cert: str | None = None ): + urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) self.url = url options = {} if cert: @@ -121,11 +123,11 @@ def add_comment_to_ticket(self, ticket_id: str, comment_message: str): """Adds a comment to the ticket Args: - comment_message (str): The body of the comment - ticket_id (str, optional): The ticket id to add the comment to. Defaults to None. + ticket_id (str): The ticket id to add the comment to. + comment_message (str): The body of the comment. """ comment = self._convert_from_html(comment_message) - return self._instance.add_comment(ticket_id, comment=comment) + self._instance.add_comment(ticket_id, body=comment) def _convert_from_html(self, string: str) -> str: string = string.replace('

Ticket: target_version=target_version ) - def add_comment_to_ticket(self, ticket_id: str, comment_message: str) -> None: + def add_comment_to_ticket(self, ticket_id: str, comment_message: str): """Add a comment to a ticket on the redmine instance Args: comment_message (str): The message to add as a comment ticket_id (str): The ticket number to add the comment to. """ - ticket = self._update_ticket( - ticket_id, notes=self._convert_html_colours(comment_message)) - return ticket + self._update_ticket(ticket_id, notes=self._convert_html_colours(comment_message)) def _update_ticket(self, ticket_id: str, **kwargs): """Writes the changed fields from the kwargs, back to the ticket @@ -140,7 +138,7 @@ def _validate_connection(self) -> bool: except (AuthError, ForbiddenError) as e: raise ConnectionError( - "Invalid Redmine API key or insufficient permssions for " + "Invalid Redmine API key or insufficient permissions for " f"{self._instance.url}." ) from e diff --git a/src/sc/review/ticketing_instances/ticket_instance_factory.py b/src/sc/review/ticketing_instances/ticket_instance_factory.py index 7ff1e01..9c27665 100644 --- a/src/sc/review/ticketing_instances/ticket_instance_factory.py +++ b/src/sc/review/ticketing_instances/ticket_instance_factory.py @@ -11,6 +11,8 @@ # 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 typing import Literal + from sc.review.exceptions import TicketIdentifierNotFound from .instances import JiraInstance, RedmineInstance from .ticketing_instance import TicketingInstance @@ -22,10 +24,28 @@ def create( provider: str, url: str, token: str, - auth_type: str = "token", + auth_type: Literal["token", "basic"] = "token", username: str | None = None, cert: str | None = None ) -> TicketingInstance: + """Ticketing instance factory method. + + Args: + provider (str): Which ticketing provider. jira or redmine supported. + url (str): URL to ticketing provider. + token (str): The token or password for the provider. + auth_type (Literal["token", "basic"], optional): Auth method. + Defaults to "token". + username (str | None, optional): Username for basic auth. Defaults to None. + cert (str | None, optional): Path to a cert. Defaults to None. + + Raises: + TicketIdentifierNotFound: Provider isn't supported. + ConnectionError: Fail to connect or validate with provider. + + Returns: + TicketingInstance + """ if provider == "redmine": return RedmineInstance( url, @@ -41,4 +61,6 @@ def create( ) else: raise TicketIdentifierNotFound( - f"Provider {provider} doesn't match any ticketing instance!") + f"Provider {provider} is not supported." + " Supported providers are: redmine, jira" + ) diff --git a/src/sc/review/ticketing_instances/ticketing_instance.py b/src/sc/review/ticketing_instances/ticketing_instance.py index 25c715d..0dd5e12 100644 --- a/src/sc/review/ticketing_instances/ticketing_instance.py +++ b/src/sc/review/ticketing_instances/ticketing_instance.py @@ -39,8 +39,7 @@ def read_ticket(self, ticket_id: str) -> Ticket: pass @abstractmethod - def add_comment_to_ticket(self, ticket_id: str, - comment_message: str) -> Ticket: + def add_comment_to_ticket(self, ticket_id: str, comment_message: str): """Abstract Method: Adds a comment to the ticket on the ticketing instance. Reads the new ticket with the new comment.