From 1654df316d96fca086929918b4c8ef21ac1cd7ee Mon Sep 17 00:00:00 2001 From: "Petr \"Stone\" Hracek" Date: Thu, 16 Jan 2025 14:52:13 +0100 Subject: [PATCH 1/8] Fix proper logging and update README.md Signed-off-by: Petr "Stone" Hracek --- README.md | 111 ++++++++++++++++++++++++++++++++++++++ auto_merger/api.py | 2 +- auto_merger/cli/merger.py | 3 +- auto_merger/merger.py | 3 +- auto_merger/pr_checker.py | 18 +++---- auto_merger/utils.py | 40 ++++---------- 6 files changed, 133 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index cea651e..e0fb7af 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,4 @@ +<<<<<<< HEAD ## auto-merger The auto-merger is tool for analysis pull request and provide feedback maintainers @@ -116,3 +117,113 @@ https://github.com/sclorg/s2i-python-container/pull/574 - Missing 2 APPROVAL In case user specifies `--send-email`, multipletimes, and the system where is auto-merger running the email service is configured, then the results are send to corresponding emails. +||||||| parent of dd96457 (Fix proper logging and update README.md) +======= +## auto-merger + +The auto-merger is tool for analysis pull request and provide feedback maintainers +what is missing in each pull request. User can specify in `.automerger.yaml` configuration file +what labels block the pull request for merging. + +The auto-merger does not count pull request that are marked as `Draft` + +The auto-merger provides two options, that are described below. + +Before running `auto-merger` you have to export GH_TOKEN, that is mandatory for command `gh` that +is used for work in GitHub. + +# Configuration file + +The configuration file should be present in `$HOME` directory where the automerger is running. + +The full example configuration file looks like: + +```yaml +github: + namespace: "sclorg" + repos: + - valkey-container + # How many approvals should have PR - default is 2 + approvals: 2 + # How many days, PR should be opened - default is 1 day + pr_lifetime: 1 + # Labels that blockes merges + blocker_labels: + - "pr/failing-ci" + - "pr/missing-review" + # Labels that should be present in pull request before merging + approval_labels: + - "READY-to-MERGE" +``` +where keys mean: +* `github` - specifies everything related to https://github.com/ pull request. +* `namespace` - specifies `namespace` in https://github.com where the pull requests will be analysed +* `repos` - specifies repositories that will be used for analysis +* `approvals` - how many approvals do you need before merging. Default is `2` +* `pr_lifetime` - how many `days` corresponding pull request should be opened. Default is `1` +* `blocker_labels` - specifies GitHub labels, that blocks pull request against merging +* `approval_labels` - specifies GitHub labels, that allows pull request merging + +# Pull Request checker + +This option is used for analysation pull request in the specific namespace and repositories mentioned +in configuration file. At the end `auto-merger` provides the output into command line. + +```bash +$ auto-merger pr-checker --help +Usage: auto-merger pr-checker [OPTIONS] + +Options: + --send-email TEXT Specify email addresses to which the mail will be sent. + --help Show this message and exit. + +``` + +The real output from `auto-merger pr-checker` could be: +```bash +s2i-ruby-container +------ + +https://github.com/sclorg/s2i-ruby-container/pull/570 pr/missing-review pr/failing-ci +https://github.com/sclorg/s2i-ruby-container/pull/569 dependencies ruby pr/missing-review + +s2i-nodejs-container +------ + +https://github.com/sclorg/s2i-nodejs-container/pull/463 pr/missing-review pr/failing-ci +https://github.com/sclorg/s2i-nodejs-container/pull/236 pr/missing-review + + +Pull requests that can be merged or missing 2 approvals +https://github.com/sclorg/s2i-python-container/pull/574 - Missing 2 APPROVAL +``` + +In case user specifies `--send-email`, multipletimes, and the system where is auto-merger running +the email service is configured, then the results are send to corresponding emails. + +# Automatic pull request merger + +This option is used for analyation pull request in the specific namespace and repositories mentioned +in configuration file. At the end `auto-merger` provides the output into command line. + +```bash +$ auto-merger merger --help +Usage: auto-merger merger [OPTIONS] + +Options: + --send-email TEXT Specify email addresses to which the mail will be sent. + --help Show this message and exit. + +``` + +The real output from `auto-merger merger` could be: +```bash +SUMMARY +https://github.com/sclorg/mariadb-container/pull/262 -> CAN BE MERGED +Let's try to merge 262.... + +``` + +In case user specifies `--send-email`, multipletimes, and the system where is auto-merger running +the email service is configured, then the results are send to corresponding emails. +>>>>>>> dd96457 (Fix proper logging and update README.md) diff --git a/auto_merger/api.py b/auto_merger/api.py index c355c62..43f1edd 100644 --- a/auto_merger/api.py +++ b/auto_merger/api.py @@ -28,7 +28,7 @@ from auto_merger.config import Config from auto_merger.merger import AutoMerger -logger = logging.getLogger("auto-merger") +logger = logging.getLogger(__name__) def pull_request_checker(config: Config, send_email: List[str]) -> int: """ diff --git a/auto_merger/cli/merger.py b/auto_merger/cli/merger.py index 58097bb..140d7d4 100644 --- a/auto_merger/cli/merger.py +++ b/auto_merger/cli/merger.py @@ -28,7 +28,8 @@ from auto_merger.config import pass_config from auto_merger import api -logger = logging.getLogger("auto-merger") + +logger = logging.getLogger(__name__) @click.command("merger") @click.option("--send-email", multiple=True, help="Specify email addresses to which the mail will be sent.") diff --git a/auto_merger/merger.py b/auto_merger/merger.py index f0652ab..b17bad3 100644 --- a/auto_merger/merger.py +++ b/auto_merger/merger.py @@ -24,6 +24,7 @@ import json +import logging import subprocess import os import shutil @@ -39,8 +40,8 @@ from auto_merger.utils import cwd from auto_merger.email import EmailSender -logger = logging.getLogger("auto-merger") +logger = logging.getLogger(__name__) class AutoMerger: container_name: str = "" diff --git a/auto_merger/pr_checker.py b/auto_merger/pr_checker.py index 2e4fe5c..c9ba9d8 100644 --- a/auto_merger/pr_checker.py +++ b/auto_merger/pr_checker.py @@ -24,6 +24,7 @@ import json +import logging import subprocess import os import shutil @@ -36,7 +37,7 @@ from auto_merger.email import EmailSender from auto_merger.config import Config -logger = logging.getLogger("auto-merger") +logger = logging.getLogger(__name__) class PRStatusChecker: @@ -50,9 +51,6 @@ def __init__(self, config: Config): self.blocking_labels = self.config.github["blocker_labels"] self.approvals = self.config.github["approvals"] self.namespace = self.config.github["namespace"] - logger.debug(f"GitHub Labels: {self.approval_labels}") - logger.debug(f"GitHub Blocking Labels: {self.blocking_labels}") - logger.debug(f"Approvals Labels: {self.approvals}") self.blocked_pr = {} self.pr_to_merge = {} self.blocked_body = [] @@ -249,7 +247,7 @@ def print_blocked_pull_request(self): # Do not print anything in case we do not have PR. if not [x for x in self.blocked_pr if self.blocked_pr[x]]: return 0 - print(f"\n\nSUMMARY\n\nPull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

") + logger.info(f"SUMMARY\n\nPull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

") self.blocked_body.append( f"Pull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

" ) @@ -257,12 +255,12 @@ def print_blocked_pull_request(self): for container, pull_requests in self.blocked_pr.items(): if not pull_requests: continue - print(f"\n{container}\n------\n") + logger.info(f"\n{container}\n------\n") self.blocked_body.append(f"{container}:") self.blocked_body.append("") for pr in pull_requests: blocked_labels = self.get_blocked_labels(pr["pr_dict"]) - print(f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} {' '.join(blocked_labels)}") + logger.info(f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} {' '.join(blocked_labels)}") self.blocked_body.append( f"" f"" @@ -273,7 +271,7 @@ def print_approval_pull_request(self): # Do not print anything in case we do not have PR. if not [x for x in self.pr_to_merge if self.pr_to_merge[x]]: return 0 - print(f"\n\nSUMMARY\n\nPull requests that can be merged approvals") + logger.info(f"SUMMARY\n\nPull requests that can be merged approvals") self.approval_body.append(f"Pull requests that can be merged or missing {self.approvals} approvals") self.approval_body.append("
Pull request URLTitleMissing labels
https://github.com/{self.namespace}/{container}/pull/{pr['number']}{pr['pr_dict']['title']}

{' '.join(blocked_labels)}

") for container, pr in self.pr_to_merge.items(): @@ -283,7 +281,7 @@ def print_approval_pull_request(self): result_pr = f"CAN BE MERGED" else: result_pr = f"Missing {self.approvals-int(pr['approvals'])} APPROVAL" - print(f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} - {result_pr}") + logger.info(f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} - {result_pr}") self.approval_body.append( f"" f"" @@ -291,7 +289,7 @@ def print_approval_pull_request(self): self.approval_body.append("
Pull request URLTitleApproval status
https://github.com/{self.namespace}/{container}/pull/{pr['number']}{pr['pr_dict']['title']}

{result_pr}


") def send_results(self, recipients): - logger.debug(f"Recepients are: {recipients}") + logger.debug(f"Recipients are: {recipients}") if not recipients: return 1 sender_class = EmailSender(recipient_email=list(recipients)) diff --git a/auto_merger/utils.py b/auto_merger/utils.py index 43a2f50..9606c68 100644 --- a/auto_merger/utils.py +++ b/auto_merger/utils.py @@ -31,7 +31,7 @@ from auto_merger.config import Config -logger = logging.getLogger("auto-merger") +logger = logging.getLogger(__name__) def run_command( @@ -79,36 +79,13 @@ def temporary_dir(prefix: str = "automerger") -> str: logger.debug(f"AutoMerger: Temporary dir name: {temp_file.name}") return temp_file.name -class AutoMergerFormatter(logging.Formatter): - def __init__( - self, - fmt=None, - datefmt=None, - *args, - ): - fmt = ( - fmt - or "%(name)s - %(levelname)s: %(message)s" - ) - datefmt = datefmt or "%Y-%m-%d %H:%M:%S" - super().__init__(fmt, datefmt, *args) - - -def setup_logger(level=logging.INFO, handler_class=logging.StreamHandler, handler_kwargs=None, datefmt: str = ""): - logger = logging.getLogger("auto-merger") - logger.setLevel(level) - logger.debug(f"Logging set to {logging.getLevelName(level)}") - - # do not re-add handlers if they are already present - if not [x for x in logger.handlers if isinstance(x, handler_class)]: - # Debug handler - handler_kwargs = handler_kwargs or {} - handler = handler_class(**handler_kwargs) - handler.setLevel(level) - - formatter = AutoMergerFormatter(None, datefmt=datefmt) - handler.setFormatter(formatter) - logger.addHandler(handler) + +def setup_logger(level=logging.INFO): + if level == logging.INFO: + format_str = "%(message)s" + else: + format_str = "%(name)s - %(levelname)s: %(message)s" + logging.basicConfig(level=level, format=format_str, handlers=[logging.StreamHandler()]) def check_mandatory_config_fields(config: Config) -> bool: @@ -127,6 +104,7 @@ def check_mandatory_config_fields(config: Config) -> bool: config_correct = False return config_correct + @contextmanager def cwd(path): """ From d11a26f29c6828c1565925b715fa51c17bc58378 Mon Sep 17 00:00:00 2001 From: "Petr \"Stone\" Hracek" Date: Thu, 16 Jan 2025 15:32:42 +0100 Subject: [PATCH 2/8] Fix logging and some pythonish issues Signed-off-by: Petr "Stone" Hracek --- auto-merger | 3 ++ auto_merger/api.py | 6 ++- auto_merger/email.py | 7 ++-- auto_merger/merger.py | 49 +++++++++++----------- auto_merger/pr_checker.py | 85 +++++++++++++++++++-------------------- 5 files changed, 76 insertions(+), 74 deletions(-) diff --git a/auto-merger b/auto-merger index e3c3fd9..acdb233 100755 --- a/auto-merger +++ b/auto-merger @@ -37,6 +37,7 @@ from auto_merger.exceptions import AutoMergerConfigException logger = logging.getLogger("auto-merger") + @click.group("auto-merger") @click.option("-d", "--debug", is_flag=True, default=False, help="Enable debug logs") @click.pass_context @@ -56,12 +57,14 @@ def auto_merger(ctx, debug): sys.exit(10) ctx.obj = c logger.debug(f"auto_merger Debug: {ctx.obj.debug}") + ctx.obj.debug = debug if ctx.obj.debug: setup_logger(level=logging.DEBUG) else: setup_logger(level=logging.INFO) logger.debug("Let's get analyses") + auto_merger.add_command(pr_checker) auto_merger.add_command(merger) diff --git a/auto_merger/api.py b/auto_merger/api.py index 43f1edd..134ffba 100644 --- a/auto_merger/api.py +++ b/auto_merger/api.py @@ -30,10 +30,12 @@ logger = logging.getLogger(__name__) + def pull_request_checker(config: Config, send_email: List[str]) -> int: """ Checks NVR from brew build against pulp """ + logger.debug(f"Configuration: {config.__str__()}") pr_status_checker = PRStatusChecker(config=config) ret_value = pr_status_checker.check_all_containers() if ret_value != 0: @@ -47,7 +49,7 @@ def pull_request_checker(config: Config, send_email: List[str]) -> int: def merger(config: Config, send_email: List[str]) -> int: - logger.debug(f"merger: {config.__str__()}") + logger.debug(f"Configuration: {config.__str__()}") auto_merger = AutoMerger(config=config) ret_value = auto_merger.check_all_containers() if ret_value != 0: @@ -57,4 +59,4 @@ def merger(config: Config, send_email: List[str]) -> int: if send_email: if not auto_merger.send_results(send_email): return 1 - return ret_value \ No newline at end of file + return ret_value diff --git a/auto_merger/email.py b/auto_merger/email.py index d3208b4..b586b05 100644 --- a/auto_merger/email.py +++ b/auto_merger/email.py @@ -23,20 +23,20 @@ import smtplib import logging -from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText from typing import List logger = logging.getLogger(__name__) + class EmailSender: def __init__(self, recipient_email: List[str] = None): self.recipient_email = recipient_email self.mime_msg = MIMEMultipart() self.send_from = "" - self.send_to = "" + self.send_to = [""] def create_email_msg(self, subject_msg: str): if not self.recipient_email: @@ -59,5 +59,4 @@ def send_email(self, subject_msg, body: List[str] = None): smtp = smtplib.SMTP("127.0.0.1") smtp.sendmail(self.send_from, self.send_to, self.mime_msg.as_string()) smtp.close() - print("Sending email finished") - + logger.info("Sending email finished") diff --git a/auto_merger/merger.py b/auto_merger/merger.py index b17bad3..e78660e 100644 --- a/auto_merger/merger.py +++ b/auto_merger/merger.py @@ -24,7 +24,6 @@ import json -import logging import subprocess import os import shutil @@ -43,6 +42,7 @@ logger = logging.getLogger(__name__) + class AutoMerger: container_name: str = "" container_dir: Path @@ -55,8 +55,8 @@ def __init__(self, config: Config): self.approvals = self.config.github["approvals"] self.namespace = self.config.github["namespace"] self.pr_lifetime = self.config.github["pr_lifetime"] - self.pr_to_merge = {} - self.approval_body = [] + self.pr_to_merge: dict = {} + self.approval_body: List = [] self.repo_data: List = [] self.temp_dir = "" @@ -86,28 +86,17 @@ def get_gh_pr_list(self): def is_authenticated(self): token = os.getenv("GH_TOKEN") if token == "": - logger.error(f"Environment variable GH_TOKEN is not specified.") + logger.error("Environment variable GH_TOKEN is not specified.") return False - cmd = [f"gh status"] + cmd = ["gh status"] logger.debug(f"Authentication command: {cmd}") try: - return_output = utils.run_command(cmd=cmd, return_output=True) + utils.run_command(cmd=cmd, return_output=True) except subprocess.CalledProcessError as cpe: logger.error(f"Authentication to GitHub failed. {cpe}") return False return True - def add_approved_pr(self, pr: {}): - self.pr_to_merge[self.container_name].append({ - "number": pr["number"], - "pr_dict": { - "title": pr["title"], - "labels": pr["labels"], - "createdAt": pr["createdAt"], - } - }) - logger.debug(f"PR {pr['number']} added to approved") - def check_labels_to_merge(self, pr): if "labels" not in pr: return False @@ -166,7 +155,10 @@ def check_pr_to_merge(self) -> bool: return False for pr in self.repo_data: if not self.check_labels_to_merge(pr): - logger.debug(f"check_pr_to_merge for {self.container_name}: pull request {pr['number']} did not met labels.") + logger.debug( + f"check_pr_to_merge for {self.container_name}: " + f"pull request {pr['number']} did not met labels." + ) continue if "reviews" not in pr: logger.debug( @@ -183,10 +175,12 @@ def check_pr_to_merge(self) -> bool: "title": pr["title"], } }) + return True def clone_repo(self): utils.run_command( - f"gh repo clone https://github.com/{self.namespace}/{self.container_name} {self.temp_dir}/{self.container_name}" + f"gh repo clone https://github.com/{self.namespace}/{self.container_name} " + f"{self.temp_dir}/{self.container_name}" ) self.container_dir = Path(self.temp_dir) / f"{self.container_name}" @@ -199,7 +193,6 @@ def merge_pull_requests(self): os.chdir(self.current_dir) self.clean_dirs() - def clean_dirs(self): os.chdir(self.current_dir) if self.container_dir.exists(): @@ -246,10 +239,11 @@ def check_all_containers(self) -> int: def print_pull_request_to_merge(self): # Do not print anything in case we do not have PR. if not [x for x in self.pr_to_merge if self.pr_to_merge[x]]: + logger.info(f"Nothing to be merged in repos {self.config.github['repos']} " + f"in organization {self.namespace}") return 0 to_approval: bool = False pr_body: List = [] - is_empty: bool = False logger.info("SUMMARY") for container, pr_list in self.pr_to_merge.items(): for pr in pr_list: @@ -258,18 +252,23 @@ def print_pull_request_to_merge(self): if int(pr["approvals"]) < self.approvals: continue to_approval = True - result_pr = f"CAN BE MERGED" + result_pr = "CAN BE MERGED" logger.info(f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} -> {result_pr}") pr_body.append( f"https://github.com/{self.namespace}/{container}/pull/{pr['number']}" f"{pr['pr_dict']['title']}

{result_pr}

" ) if to_approval: - self.approval_body.append(f"Pull requests that can be merged.") - self.approval_body.append("") + self.approval_body.append("Pull requests that can be merged.") + self.approval_body.append( + "
Pull request URLTitleApproval status
" + ) self.approval_body.extend(pr_body) self.approval_body.append("
Pull request URLTitleApproval status

") - is_empty = True + if not to_approval: + logger.info( + f"Nothing to be merged in repos {self.config.github['repos']} in organization {self.namespace}" + ) def send_results(self, recipients): logger.debug(f"Recipients are: {recipients}") diff --git a/auto_merger/pr_checker.py b/auto_merger/pr_checker.py index c9ba9d8..ff4ba2a 100644 --- a/auto_merger/pr_checker.py +++ b/auto_merger/pr_checker.py @@ -24,13 +24,12 @@ import json -import logging import subprocess import os import shutil import logging -from typing import List +from typing import List, Any from pathlib import Path from auto_merger import utils @@ -51,10 +50,10 @@ def __init__(self, config: Config): self.blocking_labels = self.config.github["blocker_labels"] self.approvals = self.config.github["approvals"] self.namespace = self.config.github["namespace"] - self.blocked_pr = {} - self.pr_to_merge = {} - self.blocked_body = [] - self.approval_body = [] + self.blocked_pr: dict = {} + self.pr_to_merge: dict = {} + self.blocked_body: List = [] + self.approval_body: List = [] self.repo_data: List = [] def is_correct_repo(self) -> bool: @@ -83,54 +82,52 @@ def get_gh_pr_list(self): def is_authenticated(self): token = os.getenv("GH_TOKEN") if token == "": - logger.error(f"Environment variable GH_TOKEN is not specified.") + logger.error("Environment variable GH_TOKEN is not specified.") return False - cmd = [f"gh status"] + cmd = ["gh status"] logger.debug(f"Authentication command: {cmd}") try: - return_output = utils.run_command(cmd=cmd, return_output=True) + utils.run_command(cmd=cmd, return_output=True) except subprocess.CalledProcessError as cpe: logger.error(f"Authentication to GitHub failed. {cpe}") return False return True - def add_blocked_pr(self, pr: {}): + def add_blocked_pull_request(self, pull_request=None) -> Any: + """ + Function adds pull request to self.blocked_pr dictionary with + specific fields like 'number' and 'pr_dict': {'title': , 'labels': } + :param pull_request: Dictionary with pull request structure + :return: + """ + if pull_request is None: + pull_request = {} present = False for stored_pr in self.blocked_pr[self.container_name]: - if int(stored_pr["number"]) == int(pr["number"]): + if int(stored_pr["number"]) == int(pull_request["number"]): present = True if present: return self.blocked_pr[self.container_name].append({ - "number": pr["number"], - "pr_dict": { - "title": pr["title"], - "labels": pr["labels"] - } - }) - logger.debug(f"PR {pr['number']} added to blocked") - - def add_approved_pr(self, pr: {}): - self.pr_to_merge[self.container_name].append({ - "number": pr["number"], + "number": pull_request["number"], "pr_dict": { - "title": pr["title"], - "labels": pr["labels"] + "title": pull_request["title"], + "labels": pull_request["labels"] } }) - logger.debug(f"PR {pr['number']} added to approved") + logger.debug(f"PR {pull_request['number']} added to blocked") + return def check_blocked_labels(self): for pr in self.repo_data: - logger.info(f"Let's check PR {pr['number']}") - logger.debug(f"Check blocked: {pr}") + logger.info(f"- Checking PR {pr['number']}") if "labels" not in pr: continue for label in pr["labels"]: if label["name"] not in self.blocking_labels: continue - logger.debug(f"Add '{pr['number']}' to blocked PRs.") - self.add_blocked_pr(pr) + logger.info(f"Add PR'{pr['number']}' to blocked PRs.") + self.add_blocked_pull_request(pull_request=pr) def check_labels_to_merge(self, pr): if "labels" not in pr: @@ -172,8 +169,6 @@ def check_pr_to_merge(self) -> bool: if len(self.repo_data) == 0: return False for pr in self.repo_data: - if PRStatusChecker.is_draft(pr): - continue logger.debug(f"PR status: {pr}") if not self.check_labels_to_merge(pr): continue @@ -187,6 +182,7 @@ def check_pr_to_merge(self) -> bool: "title": pr["title"] } } + return True def clone_repo(self): temp_dir = utils.temporary_dir() @@ -210,7 +206,7 @@ def check_all_containers(self) -> int: if not self.is_authenticated(): return 1 for container in self.config.github["repos"]: - logger.info(f"Let's check container {container}") + logger.info(f"Let's check repository in {self.namespace}/{container}") self.container_name = container self.repo_data = [] self.clone_repo() @@ -225,10 +221,6 @@ def check_all_containers(self) -> int: try: self.get_gh_pr_list() self.check_blocked_labels() - if len(self.blocked_pr[self.container_name]) != 0: - logger.info( - f"This pull request can not be merged {self.pr_to_merge}" - ) self.check_pr_to_merge() except subprocess.CalledProcessError: self.clean_dirs() @@ -237,7 +229,7 @@ def check_all_containers(self) -> int: self.clean_dirs() return 0 - def get_blocked_labels(self, pr_dict) -> List [str]: + def get_blocked_labels(self, pr_dict) -> List[str]: labels = [] for lbl in pr_dict["labels"]: labels.append(lbl["name"]) @@ -247,7 +239,9 @@ def print_blocked_pull_request(self): # Do not print anything in case we do not have PR. if not [x for x in self.blocked_pr if self.blocked_pr[x]]: return 0 - logger.info(f"SUMMARY\n\nPull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

") + logger.info( + f"SUMMARY\n\nPull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

" + ) self.blocked_body.append( f"Pull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

" ) @@ -257,13 +251,18 @@ def print_blocked_pull_request(self): continue logger.info(f"\n{container}\n------\n") self.blocked_body.append(f"{container}:") - self.blocked_body.append("") + self.blocked_body.append( + "
Pull request URLTitleMissing labels
" + ) for pr in pull_requests: blocked_labels = self.get_blocked_labels(pr["pr_dict"]) - logger.info(f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} {' '.join(blocked_labels)}") + logger.info( + f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} {' '.join(blocked_labels)}" + ) self.blocked_body.append( f"" - f"" + f"" ) self.blocked_body.append("
Pull request URLTitleMissing labels
https://github.com/{self.namespace}/{container}/pull/{pr['number']}{pr['pr_dict']['title']}

{' '.join(blocked_labels)}

{pr['pr_dict']['title']}

" + f"{' '.join(blocked_labels)}



") @@ -271,14 +270,14 @@ def print_approval_pull_request(self): # Do not print anything in case we do not have PR. if not [x for x in self.pr_to_merge if self.pr_to_merge[x]]: return 0 - logger.info(f"SUMMARY\n\nPull requests that can be merged approvals") + logger.info("SUMMARY\n\nPull requests that can be merged approvals") self.approval_body.append(f"Pull requests that can be merged or missing {self.approvals} approvals") self.approval_body.append("") for container, pr in self.pr_to_merge.items(): if not pr: continue if int(pr["approvals"]) >= self.approvals: - result_pr = f"CAN BE MERGED" + result_pr = "CAN BE MERGED" else: result_pr = f"Missing {self.approvals-int(pr['approvals'])} APPROVAL" logger.info(f"https://github.com/{self.namespace}/{container}/pull/{pr['number']} - {result_pr}") From 18ee92c6e31016014576ba528645a02f2a6e94a0 Mon Sep 17 00:00:00 2001 From: "Petr \"Stone\" Hracek" Date: Fri, 17 Jan 2025 11:38:24 +0100 Subject: [PATCH 3/8] Let's test it only on Python 3.11 and Python 3.12 Fix README.md file. Add more desciption to functions and return boolean in function that are valid for returning more functions Signed-off-by: Petr "Stone" Hracek --- .github/workflows/python-tests.yml | 2 +- README.md | 122 ----------------------------- auto_merger/merger.py | 11 +-- auto_merger/pr_checker.py | 63 +++++++++++---- tests/conftest.py | 11 +-- tests/test_config.py | 11 +-- 6 files changed, 66 insertions(+), 154 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index f698d3f..6d7fa84 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -13,7 +13,7 @@ jobs: name: Tox test strategy: matrix: - tox_env: [py38, py39, py310, py311, py312] + tox_env: [py311, py312] # Use GitHub's Linux Docker host runs-on: ubuntu-latest steps: diff --git a/README.md b/README.md index e0fb7af..ffc764a 100644 --- a/README.md +++ b/README.md @@ -1,124 +1,3 @@ -<<<<<<< HEAD -## auto-merger - -The auto-merger is tool for analysis pull request and provide feedback maintainers -what is missing in each pull request. User can specify in `.automerger.yaml` configuration file -what labels block the pull request for merging. - -The auto-merger does not count pull request that are marked as `Draft` - -The auto-merger provides two options, that are described below. - -Before running `auto-merger` you have to export GH_TOKEN, that is mandatory for command `gh` that -is used for work in GitHub. - -# Configuration file - -The configuration file should be present in `$HOME` directory where the automerger is running. - -The full example configuration file looks like: - -```yaml -github: - namespace: "sclorg" - repos: - - valkey-container - # How many approvals should have PR - default is 2 - approvals: 2 - # How many days, PR should be opened - default is 1 day - pr_lifetime: 1 - # Labels that blockes merges - blocker_labels: - - "pr/failing-ci" - - "pr/missing-review" - # Labels that should be present in pull request before merging - approval_labels: - - "READY-to-MERGE" -``` -where keys mean: -* `github` - specifies everything related to https://github.com/ pull request. -* `namespace` - specifies `namespace` in https://github.com where the pull requests will be analysed -* `repos` - specifies repositories that will be used for analysis -* `approvals` - how many approvals do you need before merging. Default is `2` -* `pr_lifetime` - how many `days` corresponding pull request should be opened. Default is `1` -* `blocker_labels` - specifies GitHub labels, that blocks pull request against merging -* `approval_labels` - specifies GitHub labels, that allows pull request merging - -# Pull Request checker - -This option is used for analysation pull request in the specific namespace and repositories mentioned -in configuration file. At the end `auto-merger` provides the output into command line. - -```bash -$ auto-merger pr-checker --help -Usage: auto-merger pr-checker [OPTIONS] - -Options: - --send-email TEXT Specify email addresses to which the mail will be sent. - --help Show this message and exit. - -``` - -The real output from `auto-merger pr-checker` could be: -```bash -s2i-ruby-container ------- - -https://github.com/sclorg/s2i-ruby-container/pull/570 pr/missing-review pr/failing-ci -https://github.com/sclorg/s2i-ruby-container/pull/569 dependencies ruby pr/missing-review - -s2i-nodejs-container ------- - -https://github.com/sclorg/s2i-nodejs-container/pull/463 pr/missing-review pr/failing-ci -https://github.com/sclorg/s2i-nodejs-container/pull/236 pr/missing-review - - -Pull requests that can be merged or missing 2 approvals -https://github.com/sclorg/s2i-python-container/pull/574 - Missing 2 APPROVAL -``` - -In case user specifies `--send-email`, multipletimes, and the system where is auto-merger running -the email service is configured, then the results are send to corresponding emails. - -# Automatic pull request merger - -This option is used for analysation pull request in the specific namespace and repositories mentioned -in configuration file. At the end `auto-merger` provides the output into command line. - -```bash -$ auto-merger pr-checker --help -Usage: auto-merger pr-checker [OPTIONS] - -Options: - --send-email TEXT Specify email addresses to which the mail will be sent. - --help Show this message and exit. - -``` - -The real output from `auto-merger pr-checker` could be: -```bash -s2i-ruby-container ------- - -https://github.com/sclorg/s2i-ruby-container/pull/570 pr/missing-review pr/failing-ci -https://github.com/sclorg/s2i-ruby-container/pull/569 dependencies ruby pr/missing-review - -s2i-nodejs-container ------- - -https://github.com/sclorg/s2i-nodejs-container/pull/463 pr/missing-review pr/failing-ci -https://github.com/sclorg/s2i-nodejs-container/pull/236 pr/missing-review - - -Pull requests that can be merged or missing 2 approvals -https://github.com/sclorg/s2i-python-container/pull/574 - Missing 2 APPROVAL -``` - -In case user specifies `--send-email`, multipletimes, and the system where is auto-merger running -the email service is configured, then the results are send to corresponding emails. -||||||| parent of dd96457 (Fix proper logging and update README.md) -======= ## auto-merger The auto-merger is tool for analysis pull request and provide feedback maintainers @@ -226,4 +105,3 @@ Let's try to merge 262.... In case user specifies `--send-email`, multipletimes, and the system where is auto-merger running the email service is configured, then the results are send to corresponding emails. ->>>>>>> dd96457 (Fix proper logging and update README.md) diff --git a/auto_merger/merger.py b/auto_merger/merger.py index e78660e..6221edf 100644 --- a/auto_merger/merger.py +++ b/auto_merger/merger.py @@ -30,9 +30,8 @@ import logging from datetime import datetime, timedelta -from typing import List from pathlib import Path - +from typing import Dict, List from auto_merger import utils from auto_merger.config import Config @@ -55,7 +54,7 @@ def __init__(self, config: Config): self.approvals = self.config.github["approvals"] self.namespace = self.config.github["namespace"] self.pr_lifetime = self.config.github["pr_lifetime"] - self.pr_to_merge: dict = {} + self.pr_to_merge: Dict = {} self.approval_body: List = [] self.repo_data: List = [] self.temp_dir = "" @@ -139,7 +138,9 @@ def is_draft(pull_request): return True return False - def check_pr_lifetime(self, pr: dict) -> bool: + def check_pr_lifetime(self, pr=None) -> bool: + if pr is None: + return False if self.pr_lifetime == 0: return True if "createdAt" not in pr: @@ -243,7 +244,7 @@ def print_pull_request_to_merge(self): f"in organization {self.namespace}") return 0 to_approval: bool = False - pr_body: List = [] + pr_body: list[str] = [] logger.info("SUMMARY") for container, pr_list in self.pr_to_merge.items(): for pr in pr_list: diff --git a/auto_merger/pr_checker.py b/auto_merger/pr_checker.py index ff4ba2a..cfe0019 100644 --- a/auto_merger/pr_checker.py +++ b/auto_merger/pr_checker.py @@ -29,7 +29,7 @@ import shutil import logging -from typing import List, Any +from typing import Any, Dict, List from pathlib import Path from auto_merger import utils @@ -50,8 +50,8 @@ def __init__(self, config: Config): self.blocking_labels = self.config.github["blocker_labels"] self.approvals = self.config.github["approvals"] self.namespace = self.config.github["namespace"] - self.blocked_pr: dict = {} - self.pr_to_merge: dict = {} + self.blocked_pr: Dict = {} + self.pr_to_merge: Dict = {} self.blocked_body: List = [] self.approval_body: List = [] self.repo_data: List = [] @@ -79,7 +79,12 @@ def get_gh_pr_list(self): continue self.repo_data.append(pr) - def is_authenticated(self): + def is_authenticated(self) -> bool: + """ + Function check if user is authenticated + :return: True if user is authenticated + False user is not authenticated + """ token = os.getenv("GH_TOKEN") if token == "": logger.error("Environment variable GH_TOKEN is not specified.") @@ -129,16 +134,28 @@ def check_blocked_labels(self): logger.info(f"Add PR'{pr['number']}' to blocked PRs.") self.add_blocked_pull_request(pull_request=pr) - def check_labels_to_merge(self, pr): + def check_labels_to_merge(self, pr) -> bool: + """ + Function checks labels for each pull request + 'label' is compared against configuration file 'github': 'blocking_labels' + :param pr: pull request dictionary with labels + :return: False is labels are not present or 'label' is int 'blocking_labels' + True pull request is approved. No blocking issue + """ if "labels" not in pr: - return True + return False for label in pr["labels"]: if label["name"] in self.blocking_labels: return False logger.debug(f"Add '{pr['number']}' to approved PRs.") return True - def check_pr_approvals(self, reviews_to_check) -> int: + def check_pr_approvals(self, reviews_to_check: dict = None) -> int: + """ + Function checks if PR has enough approvals + :param reviews_to_check: List of review to check for specific pull request + :return: Number of approvals + """ logger.debug(f"Approvals to check: {reviews_to_check}") if not reviews_to_check: return False @@ -150,7 +167,13 @@ def check_pr_approvals(self, reviews_to_check) -> int: logger.debug(f"Approval count: {approval_cnt}") return approval_cnt - def is_changes_requested(self, pr): + def is_changes_requested(self, pr) -> bool: + """ + Function checks if pull request is marked as 'changes request' + :param pr: dictionary with pull reqyest + :return: True if changes are requested + False if changes are not requested + """ if "labels" not in pr: return False for labels in pr["labels"]: @@ -159,7 +182,13 @@ def is_changes_requested(self, pr): return False @staticmethod - def is_draft(pull_request): + def is_draft(pull_request) -> bool: + """ + Function returns if pull request is draft or not. + :param pull_request: Pull request with field 'isDraft' + :return: True for draft + False not draft + """ if "isDraft" in pull_request: if pull_request["isDraft"] in ["True", "true"]: return True @@ -168,6 +197,7 @@ def is_draft(pull_request): def check_pr_to_merge(self) -> bool: if len(self.repo_data) == 0: return False + pr_to_merge: bool = False for pr in self.repo_data: logger.debug(f"PR status: {pr}") if not self.check_labels_to_merge(pr): @@ -182,7 +212,8 @@ def check_pr_to_merge(self) -> bool: "title": pr["title"] } } - return True + pr_to_merge = True + return pr_to_merge def clone_repo(self): temp_dir = utils.temporary_dir() @@ -202,9 +233,9 @@ def clean_dirs(self): if self.container_dir.exists(): shutil.rmtree(self.container_dir) - def check_all_containers(self) -> int: + def check_all_containers(self) -> bool: if not self.is_authenticated(): - return 1 + return False for container in self.config.github["repos"]: logger.info(f"Let's check repository in {self.namespace}/{container}") self.container_name = container @@ -227,7 +258,7 @@ def check_all_containers(self) -> int: logger.error(f"Something went wrong {self.container_name}.") continue self.clean_dirs() - return 0 + return True def get_blocked_labels(self, pr_dict) -> List[str]: labels = [] @@ -238,7 +269,7 @@ def get_blocked_labels(self, pr_dict) -> List[str]: def print_blocked_pull_request(self): # Do not print anything in case we do not have PR. if not [x for x in self.blocked_pr if self.blocked_pr[x]]: - return 0 + return logger.info( f"SUMMARY\n\nPull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

" ) @@ -269,7 +300,7 @@ def print_blocked_pull_request(self): def print_approval_pull_request(self): # Do not print anything in case we do not have PR. if not [x for x in self.pr_to_merge if self.pr_to_merge[x]]: - return 0 + return logger.info("SUMMARY\n\nPull requests that can be merged approvals") self.approval_body.append(f"Pull requests that can be merged or missing {self.approvals} approvals") self.approval_body.append("
Pull request URLTitleApproval status
") @@ -290,7 +321,7 @@ def print_approval_pull_request(self): def send_results(self, recipients): logger.debug(f"Recipients are: {recipients}") if not recipients: - return 1 + return sender_class = EmailSender(recipient_email=list(recipients)) subject_msg = f"Pull request statuses for organization https://github.com/{self.namespace}" sender_class.send_email(subject_msg, self.blocked_body + self.approval_body) diff --git a/tests/conftest.py b/tests/conftest.py index 3a0161a..b90df3f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -75,7 +75,7 @@ def get_pr_missing_labels_one_approval(): def get_config_dict_simple(): return { "github": { - "namespace": "sclorg", + "namespace": "foobar", "repos": [ "repo1" ], @@ -88,7 +88,7 @@ def get_config_dict_simple(): "pr/foobar1" ], # Labels that should be present in pull request before merging - "merge_labels": [ + "approval_labels": [ "ok-to-merge" ] } @@ -99,7 +99,7 @@ def get_config_dict_simple(): def get_config_dict_miss_approval_lifetime(): return { "github": { - "namespace": "sclorg", + "namespace": "foobar", "repos": [ "repo1" ], @@ -108,17 +108,18 @@ def get_config_dict_miss_approval_lifetime(): "pr/foobar1" ], # Labels that should be present in pull request before merging - "merge_labels": [ + "approval_labels": [ "ok-to-merge" ] } } + @pytest.fixture() def default_config_merger(): return { "github": { - "namespace": "sclorg", + "namespace": "foobar", "repos": [ "s2i-nodejs-container" ], diff --git a/tests/test_config.py b/tests/test_config.py index 7f318d6..cff33af 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -31,14 +31,14 @@ def get_config_simple(): config = Config() config.debug = True github_dict = { - "namespace": "sclorg", + "namespace": "foobar", "repos": ["repo1"], "approvals": 23, "pr_lifetime": 2, "blocker_labels": [ "pr/foobar1", ], - "merge_labels": [ + "approval_labels": [ "ok-to-merge" ] } @@ -46,19 +46,20 @@ def get_config_simple(): config.gitlab = None return config + @pytest.fixture() def get_config_miss_approval_and_lifetime(): config = Config() config.debug = True github_dict = { - "namespace": "sclorg", + "namespace": "foobar", "repos": ["repo1"], "approvals": 2, "pr_lifetime": 1, "blocker_labels": [ "pr/foobar1", ], - "merge_labels": [ + "approval_labels": [ "ok-to-merge" ] } @@ -67,7 +68,7 @@ def get_config_miss_approval_and_lifetime(): return config -def test_config_equal(get_config_dict_simple, get_config_simple): +def test_config_equal(get_config_simple): config = Config.get_from_dict(raw_dict=get_config_dict_simple) assert config.debug == get_config_simple.debug assert config.github == get_config_simple.github From 6d83b02734249478640dc4c2e74004c53451e40c Mon Sep 17 00:00:00 2001 From: "Petr \"Stone\" Hracek" Date: Fri, 17 Jan 2025 11:42:52 +0100 Subject: [PATCH 4/8] Fix test for configuratio file Signed-off-by: Petr "Stone" Hracek --- tests/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index cff33af..489c955 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -69,7 +69,7 @@ def get_config_miss_approval_and_lifetime(): def test_config_equal(get_config_simple): - config = Config.get_from_dict(raw_dict=get_config_dict_simple) + config = Config.get_from_dict(raw_dict=get_config_dict_simple()) assert config.debug == get_config_simple.debug assert config.github == get_config_simple.github assert config.gitlab == get_config_simple.gitlab From bc394fd8777460052d430e905783f0332c599846 Mon Sep 17 00:00:00 2001 From: "Petr \"Stone\" Hracek" Date: Fri, 17 Jan 2025 11:44:39 +0100 Subject: [PATCH 5/8] Return proper values in merger.py. Do not use int in case it is boolean Signed-off-by: Petr "Stone" Hracek --- auto_merger/merger.py | 14 +++++++------- tests/test_config.py | 5 ++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/auto_merger/merger.py b/auto_merger/merger.py index 6221edf..1e0164a 100644 --- a/auto_merger/merger.py +++ b/auto_merger/merger.py @@ -82,7 +82,7 @@ def get_gh_pr_list(self): continue self.repo_data.append(pr) - def is_authenticated(self): + def is_authenticated(self) -> bool: token = os.getenv("GH_TOKEN") if token == "": logger.error("Environment variable GH_TOKEN is not specified.") @@ -96,7 +96,7 @@ def is_authenticated(self): return False return True - def check_labels_to_merge(self, pr): + def check_labels_to_merge(self, pr) -> bool: if "labels" not in pr: return False logger.debug(f"check_labels_to_merge for {self.container_name}: {pr['labels']} and {self.approval_labels}") @@ -212,9 +212,9 @@ def merge_pr(self): logger.error(f"Merging pr {pr} failed with reason {cpe.output}") continue - def check_all_containers(self) -> int: + def check_all_containers(self) -> bool: if not self.is_authenticated(): - return 1 + return False self.temp_dir = utils.temporary_dir() for container in self.config.github["repos"]: self.container_name = container @@ -235,14 +235,14 @@ def check_all_containers(self) -> int: logger.error(f"Something went wrong {self.container_name}.") continue os.chdir(self.current_dir) - return 0 + return True def print_pull_request_to_merge(self): # Do not print anything in case we do not have PR. if not [x for x in self.pr_to_merge if self.pr_to_merge[x]]: logger.info(f"Nothing to be merged in repos {self.config.github['repos']} " f"in organization {self.namespace}") - return 0 + return to_approval: bool = False pr_body: list[str] = [] logger.info("SUMMARY") @@ -274,7 +274,7 @@ def print_pull_request_to_merge(self): def send_results(self, recipients): logger.debug(f"Recipients are: {recipients}") if not recipients: - return 1 + return False sender_class = EmailSender(recipient_email=list(recipients)) subject_msg = f"Pull request statuses for organization https://github.com/{self.namespace}" if self.approval_body: diff --git a/tests/test_config.py b/tests/test_config.py index 489c955..507c422 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -23,7 +23,6 @@ import pytest from auto_merger.config import Config -from tests.conftest import get_config_dict_simple @pytest.fixture() @@ -68,8 +67,8 @@ def get_config_miss_approval_and_lifetime(): return config -def test_config_equal(get_config_simple): - config = Config.get_from_dict(raw_dict=get_config_dict_simple()) +def test_config_equal(get_config_dict_simple, get_config_simple): + config = Config.get_from_dict(raw_dict=get_config_dict_simple) assert config.debug == get_config_simple.debug assert config.github == get_config_simple.github assert config.gitlab == get_config_simple.gitlab From 9db8651e94b697751d516fe425ddd4d4ae3c68c5 Mon Sep 17 00:00:00 2001 From: Petr Hracek Date: Tue, 21 Jan 2025 10:50:44 +0100 Subject: [PATCH 6/8] Update README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lumír 'Frenzy' Balhar --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ffc764a..67e679d 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ the email service is configured, then the results are send to corresponding emai # Automatic pull request merger -This option is used for analyation pull request in the specific namespace and repositories mentioned +This option is used for analyzation pull request in the specific namespace and repositories mentioned in configuration file. At the end `auto-merger` provides the output into command line. ```bash From 17b224f3287d20821b2ac0c58beb08b6df1056c6 Mon Sep 17 00:00:00 2001 From: Petr Hracek Date: Tue, 21 Jan 2025 12:26:14 +0100 Subject: [PATCH 7/8] Fix typing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lumír 'Frenzy' Balhar --- auto_merger/pr_checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auto_merger/pr_checker.py b/auto_merger/pr_checker.py index cfe0019..895770c 100644 --- a/auto_merger/pr_checker.py +++ b/auto_merger/pr_checker.py @@ -260,7 +260,7 @@ def check_all_containers(self) -> bool: self.clean_dirs() return True - def get_blocked_labels(self, pr_dict) -> List[str]: + def get_blocked_labels(self, pr_dict) -> list[str]: labels = [] for lbl in pr_dict["labels"]: labels.append(lbl["name"]) From affedf5006be687fd973a76acbd7e29349c5e3b0 Mon Sep 17 00:00:00 2001 From: "Petr \"Stone\" Hracek" Date: Tue, 21 Jan 2025 12:55:29 +0100 Subject: [PATCH 8/8] Fix typing the whole python programs Signed-off-by: Petr "Stone" Hracek --- .auto-merger.yaml | 2 +- .github/workflows/python-tests.yml | 2 +- .pre-commit-config.yaml | 24 ++++++++++++------- auto_merger/api.py | 6 ++--- auto_merger/cli/merger.py | 9 ++++--- auto_merger/cli/pr_checker.py | 7 ++++-- auto_merger/config.py | 13 ++++------ auto_merger/email.py | 16 ++++++++----- auto_merger/merger.py | 37 +++++++++++++++-------------- auto_merger/pr_checker.py | 38 +++++++++++++----------------- setup.py | 4 ++-- tests/conftest.py | 32 +++++++------------------ tests/test_config.py | 8 ++----- tests/test_correct_repo.py | 4 +--- tests/test_merger.py | 20 +++++++--------- tests/test_pr_status.py | 12 +++------- tox.ini | 14 +++++++++++ 17 files changed, 122 insertions(+), 126 deletions(-) diff --git a/.auto-merger.yaml b/.auto-merger.yaml index 4de10fd..ed09b88 100644 --- a/.auto-merger.yaml +++ b/.auto-merger.yaml @@ -25,4 +25,4 @@ github: approval_labels: - "READY-to-MERGE" gitlab: - namespace: "redhat/rhel/containers" \ No newline at end of file + namespace: "redhat/rhel/containers" diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 6d7fa84..a69b983 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -13,7 +13,7 @@ jobs: name: Tox test strategy: matrix: - tox_env: [py311, py312] + tox_env: [py311, py312, py313] # Use GitHub's Linux Docker host runs-on: ubuntu-latest steps: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 29bee78..48aafed 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,7 +4,7 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v2.2.3 + rev: v4.4.0 hooks: - id: check-added-large-files - id: check-ast @@ -13,11 +13,19 @@ repos: - id: detect-private-key - id: end-of-file-fixer - id: trailing-whitespace - - id: flake8 - args: - - --max-line-length=120 -- repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.711 +- repo: https://github.com/PyCQA/flake8 + rev: 6.0.0 hooks: - - id: mypy - args: [--no-strict-optional, --ignore-missing-imports] + - id: flake8 + additional_dependencies: [ + "flake8-blind-except==0.2.0", + "flake8-mutable==1.2.0", + "pep8-naming==0.11.1", + ] + args: + - --max-line-length=120 +- repo: https://github.com/psf/black + rev: 23.3.0 + hooks: + - id: black + args: [--line-length=120] diff --git a/auto_merger/api.py b/auto_merger/api.py index 134ffba..37c4abd 100644 --- a/auto_merger/api.py +++ b/auto_merger/api.py @@ -22,8 +22,6 @@ import logging -from typing import List - from auto_merger.pr_checker import PRStatusChecker from auto_merger.config import Config from auto_merger.merger import AutoMerger @@ -31,7 +29,7 @@ logger = logging.getLogger(__name__) -def pull_request_checker(config: Config, send_email: List[str]) -> int: +def pull_request_checker(config: Config, send_email: list[str] | None) -> int: """ Checks NVR from brew build against pulp """ @@ -48,7 +46,7 @@ def pull_request_checker(config: Config, send_email: List[str]) -> int: return ret_value -def merger(config: Config, send_email: List[str]) -> int: +def merger(config: Config, send_email: list[str] | None) -> int: logger.debug(f"Configuration: {config.__str__()}") auto_merger = AutoMerger(config=config) ret_value = auto_merger.check_all_containers() diff --git a/auto_merger/cli/merger.py b/auto_merger/cli/merger.py index 140d7d4..530a8a3 100644 --- a/auto_merger/cli/merger.py +++ b/auto_merger/cli/merger.py @@ -31,11 +31,14 @@ logger = logging.getLogger(__name__) + @click.command("merger") -@click.option("--send-email", multiple=True, help="Specify email addresses to which the mail will be sent.") +@click.option( + "--send-email", + multiple=True, + help="Specify email addresses to which the mail will be sent.", +) @pass_config def merger(config, send_email): - ret_value = api.merger(config=config, send_email=send_email) sys.exit(ret_value) - diff --git a/auto_merger/cli/pr_checker.py b/auto_merger/cli/pr_checker.py index 9fc2d36..3b2c53d 100644 --- a/auto_merger/cli/pr_checker.py +++ b/auto_merger/cli/pr_checker.py @@ -32,9 +32,12 @@ @click.command("pr-checker") -@click.option("--send-email", multiple=True, help="Specify email addresses to which the mail will be sent.") +@click.option( + "--send-email", + multiple=True, + help="Specify email addresses to which the mail will be sent.", +) @pass_config def pr_checker(config, send_email): ret_value = api.pull_request_checker(config=config, send_email=send_email) sys.exit(ret_value) - diff --git a/auto_merger/config.py b/auto_merger/config.py index 14ec5f4..e6d2594 100644 --- a/auto_merger/config.py +++ b/auto_merger/config.py @@ -21,26 +21,23 @@ # SOFTWARE. import logging -import yaml import click from yaml import safe_load from pathlib import Path -from typing import Dict -from auto_merger.exceptions import AutoMergerConfigException, AutoMergerNetworkException +from auto_merger.exceptions import AutoMergerConfigException logger = logging.getLogger("auto-merger") class Config: - def __init__(self): self.debug: bool = True - self.github: Dict = {} - self.gitlab: Dict = {} + self.github: dict = {} + self.gitlab: dict = {} @classmethod def get_default_config(cls) -> "Config": @@ -70,7 +67,7 @@ def get_from_dict(cls, raw_dict: dict) -> "Config": return config def __repr__(self): - return f"Config(debug={self.debug}, github={self.github}, " \ - f"gitlab={self.gitlab})" + return f"Config(debug={self.debug}, github={self.github}, " f"gitlab={self.gitlab})" + pass_config = click.make_pass_decorator(Config) diff --git a/auto_merger/email.py b/auto_merger/email.py index b586b05..e0bf5a2 100644 --- a/auto_merger/email.py +++ b/auto_merger/email.py @@ -25,14 +25,14 @@ from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText -from typing import List logger = logging.getLogger(__name__) class EmailSender: - - def __init__(self, recipient_email: List[str] = None): + def __init__(self, recipient_email=None): + if recipient_email is None: + recipient_email = [] self.recipient_email = recipient_email self.mime_msg = MIMEMultipart() self.send_from = "" @@ -50,10 +50,14 @@ def create_email_msg(self, subject_msg: str): self.mime_msg["To"] = ", ".join(self.send_to) self.mime_msg["Subject"] = subject_msg - def send_email(self, subject_msg, body: List[str] = None): + def send_email(self, subject_msg, body=None): + if body is None: + body = [] whole_body = "".join(body) - msg = ("" - f"{whole_body}") + msg = ( + "" + f"{whole_body}" + ) self.create_email_msg(subject_msg) self.mime_msg.attach(MIMEText(msg, "html")) smtp = smtplib.SMTP("127.0.0.1") diff --git a/auto_merger/merger.py b/auto_merger/merger.py index 1e0164a..4215a43 100644 --- a/auto_merger/merger.py +++ b/auto_merger/merger.py @@ -31,7 +31,6 @@ from datetime import datetime, timedelta from pathlib import Path -from typing import Dict, List from auto_merger import utils from auto_merger.config import Config @@ -54,9 +53,9 @@ def __init__(self, config: Config): self.approvals = self.config.github["approvals"] self.namespace = self.config.github["namespace"] self.pr_lifetime = self.config.github["pr_lifetime"] - self.pr_to_merge: Dict = {} - self.approval_body: List = [] - self.repo_data: List = [] + self.pr_to_merge: dict = {} + self.approval_body: list = [] + self.repo_data: list = [] self.temp_dir = "" def is_correct_repo(self) -> bool: @@ -129,6 +128,7 @@ def is_changes_requested(pr): @staticmethod def get_realtime(): from datetime import datetime + return datetime.now() @staticmethod @@ -157,25 +157,27 @@ def check_pr_to_merge(self) -> bool: for pr in self.repo_data: if not self.check_labels_to_merge(pr): logger.debug( - f"check_pr_to_merge for {self.container_name}: " - f"pull request {pr['number']} did not met labels." + f"check_pr_to_merge for {self.container_name}: " f"pull request {pr['number']} did not met labels." ) continue if "reviews" not in pr: logger.debug( f"check_pr_to_merge for {self.container_name}:" - f" pull request {pr['number']} does not have reviews yet.") + f" pull request {pr['number']} does not have reviews yet." + ) continue approval_count = self.check_pr_approvals(pr["reviews"]) if not self.check_pr_lifetime(pr=pr): continue - self.pr_to_merge[self.container_name].append({ - "number": pr["number"], - "approvals": approval_count, - "pr_dict": { - "title": pr["title"], + self.pr_to_merge[self.container_name].append( + { + "number": pr["number"], + "approvals": approval_count, + "pr_dict": { + "title": pr["title"], + }, } - }) + ) return True def clone_repo(self): @@ -240,8 +242,9 @@ def check_all_containers(self) -> bool: def print_pull_request_to_merge(self): # Do not print anything in case we do not have PR. if not [x for x in self.pr_to_merge if self.pr_to_merge[x]]: - logger.info(f"Nothing to be merged in repos {self.config.github['repos']} " - f"in organization {self.namespace}") + logger.info( + f"Nothing to be merged in repos {self.config.github['repos']} " f"in organization {self.namespace}" + ) return to_approval: bool = False pr_body: list[str] = [] @@ -267,9 +270,7 @@ def print_pull_request_to_merge(self): self.approval_body.extend(pr_body) self.approval_body.append("
Pull request URLTitleApproval status

") if not to_approval: - logger.info( - f"Nothing to be merged in repos {self.config.github['repos']} in organization {self.namespace}" - ) + logger.info(f"Nothing to be merged in repos {self.config.github['repos']} in organization {self.namespace}") def send_results(self, recipients): logger.debug(f"Recipients are: {recipients}") diff --git a/auto_merger/pr_checker.py b/auto_merger/pr_checker.py index 895770c..f438e59 100644 --- a/auto_merger/pr_checker.py +++ b/auto_merger/pr_checker.py @@ -29,7 +29,7 @@ import shutil import logging -from typing import Any, Dict, List +from typing import Any from pathlib import Path from auto_merger import utils @@ -50,11 +50,11 @@ def __init__(self, config: Config): self.blocking_labels = self.config.github["blocker_labels"] self.approvals = self.config.github["approvals"] self.namespace = self.config.github["namespace"] - self.blocked_pr: Dict = {} - self.pr_to_merge: Dict = {} - self.blocked_body: List = [] - self.approval_body: List = [] - self.repo_data: List = [] + self.blocked_pr: dict = {} + self.pr_to_merge: dict = {} + self.blocked_body: list = [] + self.approval_body: list = [] + self.repo_data: list = [] def is_correct_repo(self) -> bool: cmd = ["gh repo view --json name"] @@ -113,13 +113,15 @@ def add_blocked_pull_request(self, pull_request=None) -> Any: present = True if present: return - self.blocked_pr[self.container_name].append({ - "number": pull_request["number"], - "pr_dict": { - "title": pull_request["title"], - "labels": pull_request["labels"] + self.blocked_pr[self.container_name].append( + { + "number": pull_request["number"], + "pr_dict": { + "title": pull_request["title"], + "labels": pull_request["labels"], + }, } - }) + ) logger.debug(f"PR {pull_request['number']} added to blocked") return @@ -208,9 +210,7 @@ def check_pr_to_merge(self) -> bool: self.pr_to_merge[self.container_name] = { "number": pr["number"], "approvals": approval_count, - "pr_dict": { - "title": pr["title"] - } + "pr_dict": {"title": pr["title"]}, } pr_to_merge = True return pr_to_merge @@ -270,9 +270,7 @@ def print_blocked_pull_request(self): # Do not print anything in case we do not have PR. if not [x for x in self.blocked_pr if self.blocked_pr[x]]: return - logger.info( - f"SUMMARY\n\nPull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

" - ) + logger.info(f"SUMMARY\n\nPull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

") self.blocked_body.append( f"Pull requests that are blocked by labels [{', '.join(self.blocking_labels)}]

" ) @@ -282,9 +280,7 @@ def print_blocked_pull_request(self): continue logger.info(f"\n{container}\n------\n") self.blocked_body.append(f"{container}:") - self.blocked_body.append( - "" - ) + self.blocked_body.append("
Pull request URLTitleMissing labels
") for pr in pull_requests: blocked_labels = self.get_blocked_labels(pr["pr_dict"]) logger.info( diff --git a/setup.py b/setup.py index ead1c6a..757c4a7 100644 --- a/setup.py +++ b/setup.py @@ -30,8 +30,8 @@ this_directory = Path(__file__).parent long_description = ( - "Tool for merging sclorg pull request that meets criteria" -) # (this_directory / "README.md").read_text() + "Tool for merging sclorg pull request that meets criteria" # (this_directory / "README.md").read_text() +) def get_requirements(): diff --git a/tests/conftest.py b/tests/conftest.py index b90df3f..4c37121 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -76,21 +76,15 @@ def get_config_dict_simple(): return { "github": { "namespace": "foobar", - "repos": [ - "repo1" - ], + "repos": ["repo1"], # How many approvals should have PR "approvals": 23, # How many days, PR should be opened "pr_lifetime": 2, # Labels that blockes merges - "blocker_labels": [ - "pr/foobar1" - ], + "blocker_labels": ["pr/foobar1"], # Labels that should be present in pull request before merging - "approval_labels": [ - "ok-to-merge" - ] + "approval_labels": ["ok-to-merge"], } } @@ -100,17 +94,11 @@ def get_config_dict_miss_approval_lifetime(): return { "github": { "namespace": "foobar", - "repos": [ - "repo1" - ], + "repos": ["repo1"], # Labels that blockes merges - "blocker_labels": [ - "pr/foobar1" - ], + "blocker_labels": ["pr/foobar1"], # Labels that should be present in pull request before merging - "approval_labels": [ - "ok-to-merge" - ] + "approval_labels": ["ok-to-merge"], } } @@ -120,10 +108,8 @@ def default_config_merger(): return { "github": { "namespace": "foobar", - "repos": [ - "s2i-nodejs-container" - ], - "blocker_labels": ["pr/missing-review", 'pr/failing-ci'], - "approval_labels": ["READY-to-MERGE"] + "repos": ["s2i-nodejs-container"], + "blocker_labels": ["pr/missing-review", "pr/failing-ci"], + "approval_labels": ["READY-to-MERGE"], } } diff --git a/tests/test_config.py b/tests/test_config.py index 507c422..85f36ac 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -37,9 +37,7 @@ def get_config_simple(): "blocker_labels": [ "pr/foobar1", ], - "approval_labels": [ - "ok-to-merge" - ] + "approval_labels": ["ok-to-merge"], } config.github = github_dict config.gitlab = None @@ -58,9 +56,7 @@ def get_config_miss_approval_and_lifetime(): "blocker_labels": [ "pr/foobar1", ], - "approval_labels": [ - "ok-to-merge" - ] + "approval_labels": ["ok-to-merge"], } config.github = github_dict config.gitlab = None diff --git a/tests/test_correct_repo.py b/tests/test_correct_repo.py index c86d4e3..0ea558f 100644 --- a/tests/test_correct_repo.py +++ b/tests/test_correct_repo.py @@ -15,9 +15,7 @@ def test_get_gh_pr_correct_repo(get_repo_name, default_config_merger): def test_get_gh_pr_wrong_repo(get_repo_wrong_name, default_config_merger): - flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return( - get_repo_wrong_name - ) + flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return(get_repo_wrong_name) test_config = Config() auto_merger = PRStatusChecker(config=test_config.get_from_dict(default_config_merger)) auto_merger.container_name = "s2i-nodejs-container" diff --git a/tests/test_merger.py b/tests/test_merger.py index 6c1baa3..4879755 100644 --- a/tests/test_merger.py +++ b/tests/test_merger.py @@ -12,12 +12,10 @@ yaml_merger = { "github": { "namespace": "sclorg", - "repos": [ - "s2i-nodejs-container" - ], - "blocker_labels": ["pr/missing-review", 'pr/failing-ci'], + "repos": ["s2i-nodejs-container"], + "blocker_labels": ["pr/missing-review", "pr/failing-ci"], "approval_labels": ["READY-to-MERGE"], - "pr_lifetime": 1 + "pr_lifetime": 1, } } @@ -25,14 +23,14 @@ @pytest.mark.parametrize( "pr_created_date,pr_lifetime,return_code", ( - ("2024-12-20T09:30:11Z", 1, False), - ("2024-12-19T07:30:11Z", 1, True), - ("2024-12-21T09:30:11Z", 1, False), - ("2024-12-21T09:30:11Z", 0, True), - ) + ("2024-12-20T09:30:11Z", 1, False), + ("2024-12-19T07:30:11Z", 1, True), + ("2024-12-21T09:30:11Z", 1, False), + ("2024-12-21T09:30:11Z", 0, True), + ), ) def test_get_gh_pr_list(pr_created_date, pr_lifetime, return_code): - set_time = datetime.strptime("2024-12-20T10:35:20Z", '%Y-%m-%dT%H:%M:%SZ') + set_time = datetime.strptime("2024-12-20T10:35:20Z", "%Y-%m-%dT%H:%M:%SZ") flexmock(AutoMerger).should_receive("get_realtime").and_return(set_time) test_config = Config() auto_merger = AutoMerger(config=test_config.get_from_dict(yaml_merger)) diff --git a/tests/test_pr_status.py b/tests/test_pr_status.py index 1049408..cdc1818 100644 --- a/tests/test_pr_status.py +++ b/tests/test_pr_status.py @@ -7,9 +7,7 @@ def test_get_gh_pr_list(get_pr_missing_ci, default_config_merger): - flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return( - get_pr_missing_ci - ) + flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return(get_pr_missing_ci) test_config = Config() auto_merger = PRStatusChecker(config=test_config.get_from_dict(default_config_merger)) auto_merger.container_name = "s2i-nodejs-container" @@ -18,9 +16,7 @@ def test_get_gh_pr_list(get_pr_missing_ci, default_config_merger): def test_get_gh_two_pr_labels_missing(get_two_pr_missing_labels, default_config_merger): - flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return( - get_two_pr_missing_labels - ) + flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return(get_two_pr_missing_labels) test_config = Config() auto_merger = PRStatusChecker(config=test_config.get_from_dict(default_config_merger)) auto_merger.container_name = "s2i-nodejs-container" @@ -30,9 +26,7 @@ def test_get_gh_two_pr_labels_missing(get_two_pr_missing_labels, default_config_ def test_get_gh_pr_missing_ci(get_pr_missing_ci, default_config_merger): - flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return( - get_pr_missing_ci - ) + flexmock(PRStatusChecker).should_receive("get_gh_json_output").and_return(get_pr_missing_ci) test_config = Config() auto_merger = PRStatusChecker(config=test_config.get_from_dict(default_config_merger)) auto_merger.container_name = "s2i-nodejs-container" diff --git a/tox.ini b/tox.ini index 412f412..12350e7 100644 --- a/tox.ini +++ b/tox.ini @@ -10,3 +10,17 @@ deps = flexmock click requests + +[testenv:lint] +basepython = python3.11 +deps = + pytest + PyYAML + flexmock + click + requests +commands = + pre-commit run --all-files --show-diff-on-failure + +[flake8] +extend-ignore = E231, E702, B902
Pull request URLTitleMissing labels