From ae22b17f1724d81d894a327d4bd29ad646e39671 Mon Sep 17 00:00:00 2001 From: mashehu Date: Tue, 28 Oct 2025 14:42:01 +0100 Subject: [PATCH] don't close open template update PRs --- nf_core/pipelines/sync.py | 69 ------------------------------------ tests/pipelines/test_sync.py | 65 --------------------------------- 2 files changed, 134 deletions(-) diff --git a/nf_core/pipelines/sync.py b/nf_core/pipelines/sync.py index e977d51fc4..c3efc9e5dd 100644 --- a/nf_core/pipelines/sync.py +++ b/nf_core/pipelines/sync.py @@ -163,7 +163,6 @@ def sync(self) -> None: self.create_merge_base_branch() self.push_merge_branch() self.make_pull_request() - self.close_open_template_merge_prs() except PullRequestExceptionError as e: self.reset_target_dir() raise PullRequestExceptionError(e) @@ -410,74 +409,6 @@ def make_pull_request(self): log.debug(f"GitHub API PR worked, return code {r.status_code}") log.info(f"GitHub PR created: {self.gh_pr_returned_data['html_url']}") - def close_open_template_merge_prs(self): - """Get all template merging branches (starting with 'nf-core-template-merge-') - and check for any open PRs from these branches to the self.from_branch - If open PRs are found, add a comment and close them - """ - log.info("Checking for open PRs from template merge branches") - - # Look for existing pull-requests - list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls" - with self.gh_api.cache_disabled(): - list_prs_request = self.gh_api.get(list_prs_url) - - list_prs_json, list_prs_pp = self._parse_json_response(list_prs_request) - - log.debug(f"GitHub API listing existing PRs:\n{list_prs_url}\n{list_prs_pp}") - if list_prs_request.status_code != 200: - log.warning(f"Could not list open PRs ('{list_prs_request.status_code}')\n{list_prs_url}\n{list_prs_pp}") - return False - - for pr in list_prs_json: - if isinstance(pr, int): - log.debug(f"Incorrect PR format: {pr}") - else: - log.debug(f"Looking at PR from '{pr['head']['ref']}': {pr['html_url']}") - # Ignore closed PRs - if pr["state"] != "open": - log.debug(f"Ignoring PR as state not open ({pr['state']}): {pr['html_url']}") - continue - - # Don't close the new PR that we just opened - if pr["head"]["ref"] == self.merge_branch: - continue - - # PR is from an automated branch and goes to our target base - if pr["head"]["ref"].startswith("nf-core-template-merge-") and pr["base"]["ref"] == self.from_branch: - self.close_open_pr(pr) - - def close_open_pr(self, pr) -> bool: - """Given a PR API response, add a comment and close.""" - log.debug(f"Attempting to close PR: '{pr['html_url']}'") - - # Make a new comment explaining why the PR is being closed - comment_text = ( - f"Version `{nf_core.__version__}` of the [nf-core/tools](https://github.com/nf-core/tools) pipeline template has just been released. " - f"This pull-request is now outdated and has been closed in favour of {self.pr_url}\n\n" - f"Please use {self.pr_url} to merge in the new changes from the nf-core template as soon as possible." - ) - with self.gh_api.cache_disabled(): - self.gh_api.post(url=pr["comments_url"], data=json.dumps({"body": comment_text})) - - # Update the PR status to be closed - with self.gh_api.cache_disabled(): - pr_request = self.gh_api.patch(url=pr["url"], data=json.dumps({"state": "closed"})) - - pr_request_json, pr_request_pp = self._parse_json_response(pr_request) - - # PR update worked - if pr_request.status_code == 200: - log.debug(f"GitHub API PR-update worked:\n{pr_request_pp}") - log.info( - f"Closed GitHub PR from '{pr['head']['ref']}' to '{pr['base']['ref']}': {pr_request_json['html_url']}" - ) - return True - # Something went wrong - else: - log.warning(f"Could not close PR ('{pr_request.status_code}'):\n{pr['url']}\n{pr_request_pp}") - return False - @staticmethod def _parse_json_response(response) -> tuple[Any, str]: """Helper method to parse JSON response and create pretty-printed string. diff --git a/tests/pipelines/test_sync.py b/tests/pipelines/test_sync.py index 2c300f6a4f..4d4c5e0891 100644 --- a/tests/pipelines/test_sync.py +++ b/tests/pipelines/test_sync.py @@ -311,71 +311,6 @@ def test_make_pull_request_bad_response(self, mock_post, mock_get): "Something went badly wrong - GitHub API PR failed - got return code 404" ) - @mock.patch("nf_core.utils.gh_api.get", side_effect=mocked_requests_get) - def test_close_open_template_merge_prs(self, mock_get): - """Try closing all open prs""" - psync = nf_core.pipelines.sync.PipelineSync(self.pipeline_dir) - psync.inspect_sync_dir() - psync.get_wf_config() - psync.gh_api.get = mock_get - psync.gh_username = "list_prs" - psync.gh_repo = "list_prs/response" - os.environ["GITHUB_AUTH_TOKEN"] = "test" - - with mock.patch("nf_core.pipelines.sync.PipelineSync.close_open_pr") as mock_close_open_pr: - psync.close_open_template_merge_prs() - - prs = mock_get(f"https://api.github.com/repos/{psync.gh_repo}/pulls").data - for pr in prs: - if pr.get("state", None) == "open": - mock_close_open_pr.assert_any_call(pr) - - @mock.patch("nf_core.utils.gh_api.post", side_effect=mocked_requests_post) - @mock.patch("nf_core.utils.gh_api.patch", side_effect=mocked_requests_patch) - def test_close_open_pr(self, mock_patch, mock_post) -> None: - psync = nf_core.pipelines.sync.PipelineSync(self.pipeline_dir) - psync.inspect_sync_dir() - psync.get_wf_config() - psync.gh_api.post = mock_post - psync.gh_api.patch = mock_patch - psync.gh_username = "bad_url" - psync.gh_repo = "bad_url/response" - os.environ["GITHUB_AUTH_TOKEN"] = "test" - pr: dict[str, str | dict[str, str]] = { - "state": "open", - "head": {"ref": "nf-core-template-merge-3"}, - "base": {"ref": "main"}, - "html_url": "pr_html_url", - "url": "url_to_update_pr", - "comments_url": "pr_comments_url", - } - - assert psync.close_open_pr(pr) - mock_patch.assert_called_once_with(url="url_to_update_pr", data='{"state": "closed"}') - - @mock.patch("nf_core.utils.gh_api.post", side_effect=mocked_requests_post) - @mock.patch("nf_core.utils.gh_api.patch", side_effect=mocked_requests_patch) - def test_close_open_pr_fail(self, mock_patch, mock_post): - psync = nf_core.pipelines.sync.PipelineSync(self.pipeline_dir) - psync.inspect_sync_dir() - psync.get_wf_config() - psync.gh_api.post = mock_post - psync.gh_api.patch = mock_patch - psync.gh_username = "bad_url" - psync.gh_repo = "bad_url/response" - os.environ["GITHUB_AUTH_TOKEN"] = "test" - pr = { - "state": "open", - "head": {"ref": "nf-core-template-merge-3"}, - "base": {"ref": "main"}, - "html_url": "pr_html_url", - "url": "bad_url_to_update_pr", - "comments_url": "pr_comments_url", - } - - assert not psync.close_open_pr(pr) - mock_patch.assert_called_once_with(url="bad_url_to_update_pr", data='{"state": "closed"}') - def test_reset_target_dir(self): """Try resetting target pipeline directory""" psync = nf_core.pipelines.sync.PipelineSync(self.pipeline_dir)