From 81a964c3486592a35caf7249702c746111ac0b1d Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 15 Nov 2023 17:27:22 +0100 Subject: [PATCH 01/14] download_pr error handling --- tasks/build.py | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 7750da02..0a597406 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -345,21 +345,28 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'curl' diff for pull request # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) - clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir) + clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) + if clone_exit_code != 0: + return clone_output, clone_error, clone_exit_code git_checkout_cmd = ' '.join([ 'git checkout', branch_name, ]) checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, - "checkout branch '%s'" % branch_name, arch_job_dir) + "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) + if checkout_exit_code != 0: + return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' - curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir) - + curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) + if curl_exit_code != 0: + return curl_output, curl_error, curl_exit_code + git_apply_cmd = f'git apply {pr.number}.diff' - git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir) - + git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) + if git_apply_exit_code != 0: + return git_apply_output, git_apply_error, git_apply_exit_code def apply_cvmfs_customizations(cvmfs_customizations, arch_job_dir): """ @@ -453,7 +460,17 @@ def prepare_jobs(pr, cfg, event_info, action_filter): log(f"{fn}(): job_dir '{job_dir}'") # TODO optimisation? download once, copy and cleanup initial copy? - download_pr(base_repo_name, base_branch_name, pr, job_dir) + download_pr_output, download_pr_error, download_pr_exit_code = download_pr(base_repo_name, base_branch_name, + pr, job_dir) + + if download_pr_exit_code != 0: + download_comment = f"Download failed with error: {download_pr_error}" + download_comment = pr_comments.create_comment(repo_name=base_repo_name,pr_number=pr.number,comment=download_comment) + if download_comment: + log(f"{fn}(): created PR issue comment with id {download_comment.id}") + else: + log(f"{fn}(): failed to create PR issue comment ") + raise RuntimeError(download_pr_error) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) @@ -491,7 +508,7 @@ def prepare_job_cfg(job_dir, build_env_cfg, repos_cfg, repo_id, software_subdir, jobcfg_dir = os.path.join(job_dir, 'cfg') # create ini file job.cfg with entries: - # [site_config] + # [site_config # local_tmp = LOCAL_TMP_VALUE # shared_fs_path = SHARED_FS_PATH # build_logs_dir = BUILD_LOGS_DIR From db69f09ea86165e0382f2c55a5723562933a314a Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 10:37:17 +0100 Subject: [PATCH 02/14] Improve message and fix typo --- tasks/build.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 0a597406..682f3081 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,13 +464,15 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = f"Download failed with error: {download_pr_error}" - download_comment = pr_comments.create_comment(repo_name=base_repo_name,pr_number=pr.number,comment=download_comment) + download_comment = f"""Unable to download or merge changes between source branch and the destination branch. Error: \n + {download_pr_error}\n + Tip: This can usually be resolved by syncing your branch""" + download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") else: - log(f"{fn}(): failed to create PR issue comment ") - raise RuntimeError(download_pr_error) + log(f"{fn}(): failed to create PR issue comment") + raise RuntimeError(download_pr_error) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) @@ -508,7 +510,7 @@ def prepare_job_cfg(job_dir, build_env_cfg, repos_cfg, repo_id, software_subdir, jobcfg_dir = os.path.join(job_dir, 'cfg') # create ini file job.cfg with entries: - # [site_config + # [site_config] # local_tmp = LOCAL_TMP_VALUE # shared_fs_path = SHARED_FS_PATH # build_logs_dir = BUILD_LOGS_DIR From b688574a01ddb09beb79269ed41d0ac44cb4ebb7 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 10:40:11 +0100 Subject: [PATCH 03/14] Yet more comment formatting --- tasks/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 682f3081..f3a4c1f6 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,9 +464,9 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = f"""Unable to download or merge changes between source branch and the destination branch. Error: \n - {download_pr_error}\n - Tip: This can usually be resolved by syncing your branch""" + download_comment = f"""{download_pr_error} + Unable to download or merge changes between source branch and the destination branch. + Tip: This can usually be resolved by syncing your branch""" download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") From 1b65a3a188361e626c8079382e070e1c6f508b2d Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 10:45:15 +0100 Subject: [PATCH 04/14] More comment improvements --- tasks/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index f3a4c1f6..fd8dc240 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,9 +464,9 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = f"""{download_pr_error} - Unable to download or merge changes between source branch and the destination branch. - Tip: This can usually be resolved by syncing your branch""" + download_comment = (f"{download_pr_error}" + f"\nUnable to download or merge changes between source branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics") download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") From e3657a912f929290c3bc26c8daeb9d09733ed4f8 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Thu, 16 Nov 2023 11:06:43 +0100 Subject: [PATCH 05/14] Tiny formatting change --- tasks/build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index fd8dc240..6419d70e 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -464,9 +464,9 @@ def prepare_jobs(pr, cfg, event_info, action_filter): pr, job_dir) if download_pr_exit_code != 0: - download_comment = (f"{download_pr_error}" - f"\nUnable to download or merge changes between source branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics") + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download or merge changes between the source branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics.") download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") From e7c6d659709d5b4b3a59f30f0e4514d763ad812a Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 29 Nov 2023 09:35:42 +0100 Subject: [PATCH 06/14] Comment changes --- tasks/build.py | 45 ++++++++++++++++++++-------- tests/test_task_build.py | 64 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 13 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 6419d70e..b1da385e 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -345,6 +345,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'curl' diff for pull request # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) + print(f'cloning with command {git_clone_cmd}') clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) if clone_exit_code != 0: return clone_output, clone_error, clone_exit_code @@ -353,20 +354,51 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): 'git checkout', branch_name, ]) + print(f'checking out with command {git_checkout_cmd}') checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) if checkout_exit_code != 0: return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' + print(f'curl with command {curl_cmd}') curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) if curl_exit_code != 0: return curl_output, curl_error, curl_exit_code git_apply_cmd = f'git apply {pr.number}.diff' + print(f'git apply with command {git_apply_cmd}') git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) if git_apply_exit_code != 0: return git_apply_output, git_apply_error, git_apply_exit_code + + +def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error): + """ + Handle download_pr() exit code and write helpful comment to PR in case of failure + + Args: + base_repo_name (string): name of the repository (format USER_OR_ORGANISATION/REPOSITORY) + pr (github.PullRequest.PullRequest): instance representing the pull request + download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, + otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. + download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. + + Return: + None (implicitly). A comment is created in the appropriate PR. + + """ + if download_pr_exit_code != 0: + fn = sys._getframe().f_code.co_name + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download or merge changes between the source branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your branch and resolving any merge conflicts.") + download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) + if download_comment: + log(f"{fn}(): created PR issue comment with id {download_comment.id}") + else: + log(f"{fn}(): failed to create PR issue comment") + raise ValueError("Unable to download PR and/or sync changes") def apply_cvmfs_customizations(cvmfs_customizations, arch_job_dir): """ @@ -462,18 +494,7 @@ def prepare_jobs(pr, cfg, event_info, action_filter): # TODO optimisation? download once, copy and cleanup initial copy? download_pr_output, download_pr_error, download_pr_exit_code = download_pr(base_repo_name, base_branch_name, pr, job_dir) - - if download_pr_exit_code != 0: - download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your branch and solving any merge conflics.") - download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) - if download_comment: - log(f"{fn}(): created PR issue comment with id {download_comment.id}") - else: - log(f"{fn}(): failed to create PR issue comment") - raise RuntimeError(download_pr_error) - + comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) os_type = arch.split('/')[0] diff --git a/tests/test_task_build.py b/tests/test_task_build.py index d1dd415d..67d1b57f 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -18,6 +18,7 @@ import re import shutil from unittest.mock import patch +from unittest import TestCase # Third party imports (anything installed into the local Python environment) from collections import namedtuple @@ -25,7 +26,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job, create_pr_comment +from tasks.build import Job, create_pr_comment, comment_download_pr, download_pr from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment @@ -474,3 +475,64 @@ def test_create_read_metadata_file(mocked_github, tmpdir): job_id5 = "555" with pytest.raises(TypeError): create_metadata_file(job5, job_id5, pr_comment) + +def test_comment_download_pr(tmpdir): + """Tests for function comment_download_pr.""" + + base_repo_name = "EESSI/software-layer" + pr = MockPullRequest(999) + download_pr_exit_code = 1 + download_pr_error = "Cannot apply patch" + + # Test exception is raised if there was an error + with patch('tools.pr_comments.create_comment') as mock_download_error: + mock_download_error = "error" + with pytest.raises(Exception): + comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) + + +def test_download_pr(tmpdir): + """Tests for function download_pr.""" + + base_repo_name = "EESSI/software-layer" + pr = MockPullRequest(12) + # Test git clone error is returned properly + download_pr_exit_code = 1 + invalid_url_part = "invalid_clone_url" + output, error, exit_code = download_pr(repo_name = base_repo_name, + branch_name = invalid_url_part, pr = pr, + arch_job_dir = tmpdir) + assert exit_code == download_pr_exit_code + assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" + + # Test git checkout error is returned properly + shutil.copyfile("tests/1.diff", os.path.join(tmpdir, "1.diff")) + pr = MockPullRequest(12) + download_pr_exit_code = 1 + branch_name = "develop" + + + class CustomMock(object): + calls = 0 + def run_cmd(self, arg): + self.calls += 1 + if self.calls != 2: + return None + else: + return DEFAULT + + run_cmd.side_effect = CustomMock().run_cmd + + + + output, error, exit_code = download_pr(repo_name = base_repo_name, + branch_name = branch_name, pr = pr, + arch_job_dir = tmpdir) + + # assert exit_code == 1 + # assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" + + # Test curl error is returned properly + + # Test git apply error is returned properly + From f1ac9a4c2b7f7b6cb16cf472b03f9d9098d10ffb Mon Sep 17 00:00:00 2001 From: Pedro Santos Neves <10762799+Neves-P@users.noreply.github.com> Date: Sun, 11 Feb 2024 17:15:39 +0100 Subject: [PATCH 07/14] Revert changes to test_task_build.py --- tests/test_task_build.py | 48 +--------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-) diff --git a/tests/test_task_build.py b/tests/test_task_build.py index b2516a02..ec6c4b2b 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -26,7 +26,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job, create_pr_comment, comment_download_pr, download_pr +from tasks.build import Job, create_pr_comment from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment @@ -490,49 +490,3 @@ def test_comment_download_pr(tmpdir): with pytest.raises(Exception): comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) - -def test_download_pr(tmpdir): - """Tests for function download_pr.""" - - base_repo_name = "EESSI/software-layer" - pr = MockPullRequest(12) - # Test git clone error is returned properly - download_pr_exit_code = 1 - invalid_url_part = "invalid_clone_url" - output, error, exit_code = download_pr(repo_name = base_repo_name, - branch_name = invalid_url_part, pr = pr, - arch_job_dir = tmpdir) - assert exit_code == download_pr_exit_code - assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" - - # Test git checkout error is returned properly - shutil.copyfile("tests/1.diff", os.path.join(tmpdir, "1.diff")) - pr = MockPullRequest(12) - download_pr_exit_code = 1 - branch_name = "develop" - - - class CustomMock(object): - calls = 0 - def run_cmd(self, arg): - self.calls += 1 - if self.calls != 2: - return None - else: - return DEFAULT - - run_cmd.side_effect = CustomMock().run_cmd - - - - output, error, exit_code = download_pr(repo_name = base_repo_name, - branch_name = branch_name, pr = pr, - arch_job_dir = tmpdir) - - # assert exit_code == 1 - # assert error == "error: pathspec 'invalid_clone_url' did not match any file(s) known to git\n" - - # Test curl error is returned properly - - # Test git apply error is returned properly - From 5a96c4f01493b91e3af40f016b3657d1937ae0ec Mon Sep 17 00:00:00 2001 From: Pedro Santos Neves <10762799+Neves-P@users.noreply.github.com> Date: Sun, 11 Feb 2024 17:16:38 +0100 Subject: [PATCH 08/14] Revert changes to test_task_build.py --- tests/test_task_build.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/test_task_build.py b/tests/test_task_build.py index ec6c4b2b..60ba9adf 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -26,7 +26,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job, create_pr_comment +from tasks.build import Job from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment @@ -475,18 +475,3 @@ def test_create_read_metadata_file(mocked_github, tmpdir): job_id5 = "555" with pytest.raises(TypeError): create_metadata_file(job5, job_id5, pr_comment) - -def test_comment_download_pr(tmpdir): - """Tests for function comment_download_pr.""" - - base_repo_name = "EESSI/software-layer" - pr = MockPullRequest(999) - download_pr_exit_code = 1 - download_pr_error = "Cannot apply patch" - - # Test exception is raised if there was an error - with patch('tools.pr_comments.create_comment') as mock_download_error: - mock_download_error = "error" - with pytest.raises(Exception): - comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) - From b82cc0b267dd0e206128175b33dd4282d800f440 Mon Sep 17 00:00:00 2001 From: Pedro Santos Neves <10762799+Neves-P@users.noreply.github.com> Date: Sun, 11 Feb 2024 17:22:16 +0100 Subject: [PATCH 09/14] Revert changes to test_task_build.py --- tests/test_task_build.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_task_build.py b/tests/test_task_build.py index 60ba9adf..ea0a6692 100644 --- a/tests/test_task_build.py +++ b/tests/test_task_build.py @@ -18,7 +18,6 @@ import re import shutil from unittest.mock import patch -from unittest import TestCase # Third party imports (anything installed into the local Python environment) from collections import namedtuple @@ -26,7 +25,7 @@ import pytest # Local application imports (anything from EESSI/eessi-bot-software-layer) -from tasks.build import Job +from tasks.build import Job, create_pr_comment from tools import run_cmd, run_subprocess from tools.job_metadata import create_metadata_file, read_metadata_file from tools.pr_comments import PRComment, get_submitted_job_comment From e35f462844589b9c4d892d9632b8ceeb22a21ec6 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Mon, 12 Feb 2024 15:56:07 +0100 Subject: [PATCH 10/14] Replace `print()` with `log()` where appropriate --- tasks/build.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 86a18b58..5df959fc 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -346,7 +346,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'curl' diff for pull request # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) - print(f'cloning with command {git_clone_cmd}') + log(f'cloning with command {git_clone_cmd}') clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) if clone_exit_code != 0: return clone_output, clone_error, clone_exit_code @@ -355,20 +355,20 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): 'git checkout', branch_name, ]) - print(f'checking out with command {git_checkout_cmd}') + log(f'checking out with command {git_checkout_cmd}') checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) if checkout_exit_code != 0: return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' - print(f'curl with command {curl_cmd}') + log(f'curl with command {curl_cmd}') curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) if curl_exit_code != 0: return curl_output, curl_error, curl_exit_code git_apply_cmd = f'git apply {pr.number}.diff' - print(f'git apply with command {git_apply_cmd}') + log(f'git apply with command {git_apply_cmd}') git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) if git_apply_exit_code != 0: return git_apply_output, git_apply_error, git_apply_exit_code From d92e1da79b9b961b1b696cdeae688f8442bd0f80 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Mon, 12 Feb 2024 16:31:33 +0100 Subject: [PATCH 11/14] Lint code --- tasks/build.py | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 5df959fc..17ea3ad2 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -347,7 +347,9 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): # - 'git apply' diff file git_clone_cmd = ' '.join(['git clone', f'https://github.com/{repo_name}', arch_job_dir]) log(f'cloning with command {git_clone_cmd}') - clone_output, clone_error, clone_exit_code = run_cmd(git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False) + clone_output, clone_error, clone_exit_code = run_cmd( + git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False + ) if clone_exit_code != 0: return clone_output, clone_error, clone_exit_code @@ -356,23 +358,28 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): branch_name, ]) log(f'checking out with command {git_checkout_cmd}') - checkout_output, checkout_err, checkout_exit_code = run_cmd(git_checkout_cmd, - "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False) + checkout_output, checkout_err, checkout_exit_code = run_cmd( + git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False + ) if checkout_exit_code != 0: - return checkout_output, checkout_err, checkout_exit_code + return checkout_output, checkout_err, checkout_exit_code curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' log(f'curl with command {curl_cmd}') - curl_output, curl_error, curl_exit_code = run_cmd(curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False) + curl_output, curl_error, curl_exit_code = run_cmd( + curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False + ) if curl_exit_code != 0: - return curl_output, curl_error, curl_exit_code - + return curl_output, curl_error, curl_exit_code + git_apply_cmd = f'git apply {pr.number}.diff' log(f'git apply with command {git_apply_cmd}') - git_apply_output, git_apply_error, git_apply_exit_code = run_cmd(git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False) + git_apply_output, git_apply_error, git_apply_exit_code = run_cmd( + git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False + ) if git_apply_exit_code != 0: return git_apply_output, git_apply_error, git_apply_exit_code - + def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error): """ @@ -381,26 +388,31 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e Args: base_repo_name (string): name of the repository (format USER_OR_ORGANISATION/REPOSITORY) pr (github.PullRequest.PullRequest): instance representing the pull request - download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, - otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. - download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. + download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, + otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. + download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. Return: None (implicitly). A comment is created in the appropriate PR. - + """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your branch and resolving any merge conflicts.") - download_comment = pr_comments.create_comment(repo_name=base_repo_name, pr_number=pr.number, comment=download_comment) + f"\nUnable to download or merge changes between the source" + " branch and the destination branch.\n" + f"\nTip: This can usually be resolved by syncing your" + " branch and resolving any merge conflicts.") + download_comment = pr_comments.create_comment( + repo_name=base_repo_name, pr_number=pr.number, comment=download_comment + ) if download_comment: log(f"{fn}(): created PR issue comment with id {download_comment.id}") else: log(f"{fn}(): failed to create PR issue comment") raise ValueError("Unable to download PR and/or sync changes") + def apply_cvmfs_customizations(cvmfs_customizations, arch_job_dir): """ Apply cvmfs_customizations to job From 3213fbe3d23dd81768ca459119005077452080c3 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Mon, 12 Feb 2024 17:07:28 +0100 Subject: [PATCH 12/14] Handle each error stage individually --- tasks/build.py | 54 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/tasks/build.py b/tasks/build.py index 17ea3ad2..e5845737 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -337,7 +337,9 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): arch_job_dir (string): working directory of the job to be submitted Returns: - None (implicitly) + None (implicitly), in case an error is caught in the git clone, git checkout, curl, + or git apply commands, returns the output, stderror, exit code and a string + stating which of these commands failed. """ # download pull request to arch_job_dir # - 'git clone' repository into arch_job_dir (NOTE 'git clone' requires that @@ -351,7 +353,8 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False ) if clone_exit_code != 0: - return clone_output, clone_error, clone_exit_code + error_stage = 'git clone' + return clone_output, clone_error, clone_exit_code, error_stage git_checkout_cmd = ' '.join([ 'git checkout', @@ -362,7 +365,8 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False ) if checkout_exit_code != 0: - return checkout_output, checkout_err, checkout_exit_code + error_stage = 'git checkout' + return checkout_output, checkout_err, checkout_exit_code, error_stage curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' log(f'curl with command {curl_cmd}') @@ -370,7 +374,8 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False ) if curl_exit_code != 0: - return curl_output, curl_error, curl_exit_code + error_stage = "curl" + return curl_output, curl_error, curl_exit_code, error_stage git_apply_cmd = f'git apply {pr.number}.diff' log(f'git apply with command {git_apply_cmd}') @@ -378,10 +383,11 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False ) if git_apply_exit_code != 0: - return git_apply_output, git_apply_error, git_apply_exit_code + error_stage = 'git apply' + return git_apply_output, git_apply_error, git_apply_exit_code, error_stage -def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error): +def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error, error_stage): """ Handle download_pr() exit code and write helpful comment to PR in case of failure @@ -391,6 +397,8 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e download_pr_exit_code (int): exit code from download_pr(). 0 if all tasks were successful, otherwise it corresponds to the error codes of git clone, git checkout, git apply, or curl. download_pr_error (string): none, or the output of stderr from git clone, git checkout, git apply or curl. + error_stage (string): a string informing the stage where download_pr() failed. Can be 'git clone', + 'git checkout', 'curl', or 'git apply'. Return: None (implicitly). A comment is created in the appropriate PR. @@ -398,11 +406,28 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name - download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source" - " branch and the destination branch.\n" - f"\nTip: This can usually be resolved by syncing your" - " branch and resolving any merge conflicts.") + if error_stage == 'git clone': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to clone the target repository." + f"\n_Tip: This could be a connection failure." + f" Try again and if the issue remains check if the address is correct_.") + elif error_stage == 'git checkout': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to checkout to the correct branch." + f"\n_Tip: Ensure that the branch name is correct and the target" + " branch is available._") + elif error_stage == 'curl': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download the .diff file." + f"\n_Tip: This could be a connection failure." + f" Try again and if the issue remains check if the address is correct_") + elif error_stage == 'git apply': + download_comment = (f"`{download_pr_error}`" + f"\nUnable to download or merge changes between the source" + f" branch and the destination branch.\n" + f"\n_Tip: This can usually be resolved by syncing your" + f" branch and resolving any merge conflicts._") + download_comment = pr_comments.create_comment( repo_name=base_repo_name, pr_number=pr.number, comment=download_comment ) @@ -505,9 +530,10 @@ def prepare_jobs(pr, cfg, event_info, action_filter): log(f"{fn}(): job_dir '{job_dir}'") # TODO optimisation? download once, copy and cleanup initial copy? - download_pr_output, download_pr_error, download_pr_exit_code = download_pr(base_repo_name, base_branch_name, - pr, job_dir) - comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error) + download_pr_output, download_pr_error, download_pr_exit_code, error_stage = download_pr( + base_repo_name, base_branch_name, pr, job_dir + ) + comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_error, error_stage) # prepare job configuration file 'job.cfg' in directory /cfg cpu_target = '/'.join(arch.split('/')[1:]) os_type = arch.split('/')[0] From 570217f8238924f03227ac6902f511830475ffe0 Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 14 Feb 2024 16:01:20 +0100 Subject: [PATCH 13/14] Use globaly defined string constants for errors --- app.cfg.example | 2 ++ tasks/build.py | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app.cfg.example b/app.cfg.example index 7a6a1073..57d09bbb 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -219,3 +219,5 @@ no_matching_tarball = No tarball matching `{tarball_pattern}` found in job dir. multiple_tarballs = Found {num_tarballs} tarballs in job dir - only 1 matching `{tarball_pattern}` expected. job_result_unknown_fmt =
:shrug: UNKNOWN _(click triangle for detailed information)_
  • Job results file `{filename}` does not exist in job directory, or parsing it failed.
  • No artefacts were found/reported.
job_test_unknown_fmt =
:shrug: UNKNOWN _(click triangle for detailed information)_
  • Job test file `{filename}` does not exist in job directory, or parsing it failed.
+ +[download_pr_comments] diff --git a/tasks/build.py b/tasks/build.py index e5845737..87f0cd2d 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -42,6 +42,10 @@ CONTAINER_CACHEDIR = "container_cachedir" CVMFS_CUSTOMIZATIONS = "cvmfs_customizations" DEFAULT_JOB_TIME_LIMIT = "24:00:00" +ERROR_CURL = "curl" +ERROR_GIT_APPLY = "git apply" +ERROR_GIT_CHECKOUT = "git checkout" +ERROR_GIT_CLONE = "curl" GITHUB = "github" HTTPS_PROXY = "https_proxy" HTTP_PROXY = "http_proxy" @@ -353,7 +357,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_clone_cmd, "Clone repo", arch_job_dir, raise_on_error=False ) if clone_exit_code != 0: - error_stage = 'git clone' + error_stage = ERROR_GIT_CLONE return clone_output, clone_error, clone_exit_code, error_stage git_checkout_cmd = ' '.join([ @@ -365,7 +369,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_checkout_cmd, "checkout branch '%s'" % branch_name, arch_job_dir, raise_on_error=False ) if checkout_exit_code != 0: - error_stage = 'git checkout' + error_stage = ERROR_GIT_CHECKOUT return checkout_output, checkout_err, checkout_exit_code, error_stage curl_cmd = f'curl -L https://github.com/{repo_name}/pull/{pr.number}.diff > {pr.number}.diff' @@ -374,7 +378,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): curl_cmd, "Obtain patch", arch_job_dir, raise_on_error=False ) if curl_exit_code != 0: - error_stage = "curl" + error_stage = ERROR_CURL return curl_output, curl_error, curl_exit_code, error_stage git_apply_cmd = f'git apply {pr.number}.diff' @@ -383,7 +387,7 @@ def download_pr(repo_name, branch_name, pr, arch_job_dir): git_apply_cmd, "Apply patch", arch_job_dir, raise_on_error=False ) if git_apply_exit_code != 0: - error_stage = 'git apply' + error_stage = ERROR_GIT_APPLY return git_apply_output, git_apply_error, git_apply_exit_code, error_stage @@ -406,22 +410,22 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name - if error_stage == 'git clone': + if error_stage == ERROR_GIT_CLONE: download_comment = (f"`{download_pr_error}`" f"\nUnable to clone the target repository." f"\n_Tip: This could be a connection failure." f" Try again and if the issue remains check if the address is correct_.") - elif error_stage == 'git checkout': + elif error_stage == ERROR_GIT_CHECKOUT: download_comment = (f"`{download_pr_error}`" f"\nUnable to checkout to the correct branch." f"\n_Tip: Ensure that the branch name is correct and the target" " branch is available._") - elif error_stage == 'curl': + elif error_stage == ERROR_CURL: download_comment = (f"`{download_pr_error}`" f"\nUnable to download the .diff file." f"\n_Tip: This could be a connection failure." f" Try again and if the issue remains check if the address is correct_") - elif error_stage == 'git apply': + elif error_stage == ERROR_GIT_APPLY: download_comment = (f"`{download_pr_error}`" f"\nUnable to download or merge changes between the source" f" branch and the destination branch.\n" From fd6cda1239cbf48bf7d5fa250b8f2c99dd80738a Mon Sep 17 00:00:00 2001 From: Neves-P Date: Wed, 14 Feb 2024 16:36:09 +0100 Subject: [PATCH 14/14] Move text snippets to cfg_file --- app.cfg.example | 8 ++++++++ tasks/build.py | 32 +++++++++++++++++++------------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/app.cfg.example b/app.cfg.example index 57d09bbb..1888e499 100644 --- a/app.cfg.example +++ b/app.cfg.example @@ -221,3 +221,11 @@ job_result_unknown_fmt =
:shrug: UNKNOWN _(click triangle for job_test_unknown_fmt =
:shrug: UNKNOWN _(click triangle for detailed information)_
  • Job test file `{filename}` does not exist in job directory, or parsing it failed.
[download_pr_comments] +git_clone_failure = Unable to clone the target repository. +git_clone_tip = _Tip: This could be a connection failure. Try again and if the issue remains check if the address is correct_. +git_checkout_failure = Unable to checkout to the correct branch. +git_checkout_tip = _Tip: Ensure that the branch name is correct and the target branch is available._ +curl_failure = Unable to download the `.diff` file. +curl_tip = _Tip: This could be a connection failure. Try again and if the issue remains check if the address is correct_ +git_apply_failure = Unable to download or merge changes between the source branch and the destination branch. +git_apply_tip = _Tip: This can usually be resolved by syncing your branch and resolving any merge conflicts._ diff --git a/tasks/build.py b/tasks/build.py index 87f0cd2d..473599b6 100644 --- a/tasks/build.py +++ b/tasks/build.py @@ -40,13 +40,22 @@ BUILD_PERMISSION = "build_permission" CFG_DIRNAME = "cfg" CONTAINER_CACHEDIR = "container_cachedir" +CURL_FAILURE = "curl_failure" +CURL_TIP = "curl_tip" CVMFS_CUSTOMIZATIONS = "cvmfs_customizations" DEFAULT_JOB_TIME_LIMIT = "24:00:00" +DOWNLOAD_PR_COMMENTS = "download_pr_comments" ERROR_CURL = "curl" ERROR_GIT_APPLY = "git apply" ERROR_GIT_CHECKOUT = "git checkout" ERROR_GIT_CLONE = "curl" GITHUB = "github" +GIT_CLONE_FAILURE = "git_clone_failure" +GIT_CLONE_TIP = "git_clone_tip" +GIT_CHECKOUT_FAILURE = "git_checkout_failure" +GIT_CHECKOUT_TIP = "git_checkout_tip" +GIT_APPLY_FAILURE = "git_apply_failure" +GIT_APPLY_TIP = "git_apply_tip" HTTPS_PROXY = "https_proxy" HTTP_PROXY = "http_proxy" INITIAL_COMMENT = "initial_comment" @@ -410,27 +419,24 @@ def comment_download_pr(base_repo_name, pr, download_pr_exit_code, download_pr_e """ if download_pr_exit_code != 0: fn = sys._getframe().f_code.co_name + + download_pr_comments_cfg = config.read_config()[DOWNLOAD_PR_COMMENTS] if error_stage == ERROR_GIT_CLONE: download_comment = (f"`{download_pr_error}`" - f"\nUnable to clone the target repository." - f"\n_Tip: This could be a connection failure." - f" Try again and if the issue remains check if the address is correct_.") + f"{download_pr_comments_cfg[GIT_CLONE_FAILURE]}" + f"{download_pr_comments_cfg[GIT_CLONE_TIP]}") elif error_stage == ERROR_GIT_CHECKOUT: download_comment = (f"`{download_pr_error}`" - f"\nUnable to checkout to the correct branch." - f"\n_Tip: Ensure that the branch name is correct and the target" - " branch is available._") + f"{download_pr_comments_cfg[GIT_CHECKOUT_FAILURE]}" + f"{download_pr_comments_cfg[GIT_CHECKOUT_TIP]}") elif error_stage == ERROR_CURL: download_comment = (f"`{download_pr_error}`" - f"\nUnable to download the .diff file." - f"\n_Tip: This could be a connection failure." - f" Try again and if the issue remains check if the address is correct_") + f"{download_pr_comments_cfg[CURL_FAILURE]}" + f"{download_pr_comments_cfg[CURL_TIP]}") elif error_stage == ERROR_GIT_APPLY: download_comment = (f"`{download_pr_error}`" - f"\nUnable to download or merge changes between the source" - f" branch and the destination branch.\n" - f"\n_Tip: This can usually be resolved by syncing your" - f" branch and resolving any merge conflicts._") + f"{download_pr_comments_cfg[GIT_APPLY_FAILURE]}" + f"{download_pr_comments_cfg[GIT_APPLY_TIP]}") download_comment = pr_comments.create_comment( repo_name=base_repo_name, pr_number=pr.number, comment=download_comment