Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 0 additions & 69 deletions nf_core/pipelines/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't remove this functions, maybe we want to reimplement this in the future? But we could deprecate them for now, maybe add a TODO comment or something similar?

"""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.
Expand Down
65 changes: 0 additions & 65 deletions tests/pipelines/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down