From 66747515284cf144a438e1ca7443aa7cc9451841 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Thu, 7 Jan 2021 13:45:52 +0100 Subject: [PATCH 01/36] initial commit new sync branch --- nf_core/sync.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/nf_core/sync.py b/nf_core/sync.py index 959648ae03..49f26d712b 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -239,6 +239,11 @@ def push_template_branch(self): except git.exc.GitCommandError as e: raise PullRequestException("Could not push TEMPLATE branch:\n {}".format(e)) + def create_merge_base_branch(self): + """Checkout a new branch from the updated TEMPLATE branch + This branch will then be used to create the PR + """ + def make_pull_request(self): """Create a pull request to a base branch (default: dev), from a head branch (default: TEMPLATE) From 88315c59453a1d8c9b78b9934a488760bdc5520a Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Thu, 7 Jan 2021 13:52:16 +0100 Subject: [PATCH 02/36] added function barebones --- nf_core/sync.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nf_core/sync.py b/nf_core/sync.py index 49f26d712b..9942462973 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -239,10 +239,17 @@ def push_template_branch(self): except git.exc.GitCommandError as e: raise PullRequestException("Could not push TEMPLATE branch:\n {}".format(e)) + def close_open_merge_pull_requests(self): + """Close any open merger PRs""" + # TODO implement this function + return None + def create_merge_base_branch(self): """Checkout a new branch from the updated TEMPLATE branch This branch will then be used to create the PR """ + # TODO implement this function + return None def make_pull_request(self): """Create a pull request to a base branch (default: dev), From 80947b21834e4bb8ae5d5cb0199808b7b675c6e9 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Thu, 7 Jan 2021 15:47:07 +0100 Subject: [PATCH 03/36] create sync PRs from extra branch instead of TEMPLATE --- nf_core/sync.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 9942462973..8c1b1724ff 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -67,6 +67,7 @@ def __init__( self.pipeline_dir = os.path.abspath(pipeline_dir) self.from_branch = from_branch self.original_branch = None + self.merge_branch = "nf-core-template-merge-{}".format(nf_core.__version__) self.made_changes = False self.make_pr = make_pr self.gh_pr_returned_data = {} @@ -95,6 +96,8 @@ def sync(self): if self.made_changes and self.make_pr: try: self.push_template_branch() + self.create_merge_base_branch() + self.push_merge_branch() self.make_pull_request() except PullRequestException as e: self.reset_target_dir() @@ -248,8 +251,20 @@ def create_merge_base_branch(self): """Checkout a new branch from the updated TEMPLATE branch This branch will then be used to create the PR """ - # TODO implement this function - return None + log.info("Checking out merge base branch {}".format(self.merge_branch)) + try: + self.repo.create_head(self.merge_branch) + except git.exc.GitCommandError: + raise SyncException("Could not checkout branch '{}'".format(self.merge_branch)) + + def push_merge_branch(self): + """Push the newly create merge branch to the remote repository""" + log.info("Pushing {} branch to remote".format(self.merge_branch)) + try: + origin = self.repo.remote() + origin.push(self.merge_branch) + except git.exc.GitCommandError as e: + raise PullRequestException("Could not push {} branch:\n {}".format(self.merge_branch, e)) def make_pull_request(self): """Create a pull request to a base branch (default: dev), @@ -366,7 +381,7 @@ def submit_pull_request(self, pr_title, pr_body_text): "title": pr_title, "body": pr_body_text, "maintainer_can_modify": True, - "head": "TEMPLATE", + "head": self.merge_branch, "base": self.from_branch, } From 3e1cb9149691030f1ee4e227b7c5a66e48ed2baa Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Thu, 7 Jan 2021 16:19:02 +0100 Subject: [PATCH 04/36] looking for open PRs --- nf_core/sync.py | 69 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 8c1b1724ff..056b50cfc9 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -67,7 +67,7 @@ def __init__( self.pipeline_dir = os.path.abspath(pipeline_dir) self.from_branch = from_branch self.original_branch = None - self.merge_branch = "nf-core-template-merge-{}".format(nf_core.__version__) + self.merge_branch = "nf-core-template-merge-3{}".format(nf_core.__version__) self.made_changes = False self.make_pr = make_pr self.gh_pr_returned_data = {} @@ -91,6 +91,7 @@ def sync(self): self.delete_template_branch_files() self.make_template_pipeline() self.commit_template_changes() + self.close_open_merge_pull_requests() # Push and make a pull request if we've been asked to if self.made_changes and self.make_pr: @@ -243,9 +244,71 @@ def push_template_branch(self): raise PullRequestException("Could not push TEMPLATE branch:\n {}".format(e)) def close_open_merge_pull_requests(self): + # TODO: implement this function """Close any open merger PRs""" - # TODO implement this function - return None + assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" + log.info("Checking for open PRs from template merge branches") + # Get list of all branches + branch_list = self.repo.branches + branch_list = [b.name for b in branch_list] + # Get any merging branches + branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge")] + + # For each branch, check if a PR is open to 'from_branch'. If yes - close it + for branch in branch_list: + log.info("Checking branch: {}".format(branch)) + # Look for existing pull-requests + list_prs_url = "https://api.github.com/repos/{}/pulls?head=nf-core:{}&base={}".format( + self.gh_repo, branch, self.from_branch + ) + r = requests.get( + url=list_prs_url, + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + ) + try: + r_json = json.loads(r.content) + r_pp = json.dumps(r_json, indent=4) + except: + r_json = r.content + r_pp = r.content + + if r.status_code == 200: + log.debug("GitHub API listing existing PRs:\n{}".format(r_pp)) + + # No open PRs + if len(r_json) == 0: + log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) + return False + + # Close existing PR + pr_update_api_url = r_json[0]["url"] + pr_content = {"state": "closed"} + + r = requests.patch( + url=pr_update_api_url, + data=json.dumps(pr_content), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + ) + try: + r_json = json.loads(r.content) + r_pp = json.dumps(r_json, indent=4) + except: + r_json = r.content + r_pp = r.content + + # PR update worked + if r.status_code == 200: + log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) + log.info("Updated GitHub PR: {}".format(r_json["html_url"])) + return True + # Something went wrong + else: + log.warning("Could not update PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp)) + return False + + # Something went wrong + else: + log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) def create_merge_base_branch(self): """Checkout a new branch from the updated TEMPLATE branch From 23f8169776c570e82e145b774016f479fe5bedf1 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 8 Jan 2021 08:54:22 +0100 Subject: [PATCH 05/36] sucessfully closing open PRs --- nf_core/sync.py | 89 +++++++++++++++++++++++++------------------------ 1 file changed, 46 insertions(+), 43 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 056b50cfc9..a9995641d8 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -67,7 +67,7 @@ def __init__( self.pipeline_dir = os.path.abspath(pipeline_dir) self.from_branch = from_branch self.original_branch = None - self.merge_branch = "nf-core-template-merge-3{}".format(nf_core.__version__) + self.merge_branch = "nf-core-template-merge-8{}".format(nf_core.__version__) self.made_changes = False self.make_pr = make_pr self.gh_pr_returned_data = {} @@ -253,16 +253,43 @@ def close_open_merge_pull_requests(self): branch_list = [b.name for b in branch_list] # Get any merging branches branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge")] - - # For each branch, check if a PR is open to 'from_branch'. If yes - close it for branch in branch_list: log.info("Checking branch: {}".format(branch)) - # Look for existing pull-requests - list_prs_url = "https://api.github.com/repos/{}/pulls?head=nf-core:{}&base={}".format( - self.gh_repo, branch, self.from_branch - ) - r = requests.get( - url=list_prs_url, + + self._close_open_pr(branch) + + def _close_open_pr(self, branch): + log.info("Checking branch: {}".format(branch)) + # Look for existing pull-requests + list_prs_url = "https://api.github.com/repos/{}/pulls?head={}&base={}".format( + self.gh_repo, branch, self.from_branch + ) + r = requests.get( + url=list_prs_url, + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + ) + try: + r_json = json.loads(r.content) + r_pp = json.dumps(r_json, indent=4) + except: + r_json = r.content + r_pp = r.content + + if r.status_code == 200: + log.debug("GitHub API listing existing PRs:\n{}".format(r_pp)) + + # No open PRs + if len(r_json) == 0: + log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) + return False + + # Close existing PR + pr_update_api_url = r_json[0]["url"] + pr_content = {"state": "closed"} + + r = requests.patch( + url=pr_update_api_url, + data=json.dumps(pr_content), auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), ) try: @@ -272,43 +299,19 @@ def close_open_merge_pull_requests(self): r_json = r.content r_pp = r.content + # PR update worked if r.status_code == 200: - log.debug("GitHub API listing existing PRs:\n{}".format(r_pp)) - - # No open PRs - if len(r_json) == 0: - log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) - return False - - # Close existing PR - pr_update_api_url = r_json[0]["url"] - pr_content = {"state": "closed"} - - r = requests.patch( - url=pr_update_api_url, - data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), - ) - try: - r_json = json.loads(r.content) - r_pp = json.dumps(r_json, indent=4) - except: - r_json = r.content - r_pp = r.content - - # PR update worked - if r.status_code == 200: - log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) - log.info("Updated GitHub PR: {}".format(r_json["html_url"])) - return True - # Something went wrong - else: - log.warning("Could not update PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp)) - return False - + log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) + log.info("Updated GitHub PR: {}".format(r_json["html_url"])) + return True # Something went wrong else: - log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) + log.warning("Could not update PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp)) + return False + + # Something went wrong + else: + log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) def create_merge_base_branch(self): """Checkout a new branch from the updated TEMPLATE branch From ac415535e8d5426d5ba1794dea03388318df7ab9 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 8 Jan 2021 12:11:23 +0100 Subject: [PATCH 06/36] delete PR update function --- nf_core/sync.py | 103 ++++++++++-------------------------------------- 1 file changed, 21 insertions(+), 82 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index a9995641d8..2eb0682ba2 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -67,7 +67,7 @@ def __init__( self.pipeline_dir = os.path.abspath(pipeline_dir) self.from_branch = from_branch self.original_branch = None - self.merge_branch = "nf-core-template-merge-8{}".format(nf_core.__version__) + self.merge_branch = "nf-core-template-merge-11{}".format(nf_core.__version__) self.made_changes = False self.make_pr = make_pr self.gh_pr_returned_data = {} @@ -87,11 +87,11 @@ def sync(self): self.inspect_sync_dir() self.get_wf_config() + self.close_open_template_merge_pull_requests() self.checkout_template_branch() self.delete_template_branch_files() self.make_template_pipeline() self.commit_template_changes() - self.close_open_merge_pull_requests() # Push and make a pull request if we've been asked to if self.made_changes and self.make_pr: @@ -243,22 +243,26 @@ def push_template_branch(self): except git.exc.GitCommandError as e: raise PullRequestException("Could not push TEMPLATE branch:\n {}".format(e)) - def close_open_merge_pull_requests(self): - # TODO: implement this function - """Close any open merger PRs""" + def close_open_template_merge_pull_requests(self): + """Get all template merging branches (start with 'nf-core-template-merge-') + and check of any open PRs from these branches to the self.from_branch + If open PRs are found, close them + """ assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" log.info("Checking for open PRs from template merge branches") # Get list of all branches branch_list = self.repo.branches branch_list = [b.name for b in branch_list] - # Get any merging branches - branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge")] + # Subset to template merging branches + branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge-")] for branch in branch_list: - log.info("Checking branch: {}".format(branch)) - - self._close_open_pr(branch) + # Check for open PRs and close if found + self.close_open_pr(branch) - def _close_open_pr(self, branch): + def close_open_pr(self, branch): + """Given a branch, check for open PRs from that branch to self.from_branch + and close if PRs have been found + """ log.info("Checking branch: {}".format(branch)) # Look for existing pull-requests list_prs_url = "https://api.github.com/repos/{}/pulls?head={}&base={}".format( @@ -302,11 +306,11 @@ def _close_open_pr(self, branch): # PR update worked if r.status_code == 200: log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) - log.info("Updated GitHub PR: {}".format(r_json["html_url"])) + log.info("Closed GitHub PR: {}".format(r_json["html_url"])) return True # Something went wrong else: - log.warning("Could not update PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp)) + log.warning("Could not close PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp)) return False # Something went wrong @@ -314,14 +318,14 @@ def _close_open_pr(self, branch): log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) def create_merge_base_branch(self): - """Checkout a new branch from the updated TEMPLATE branch + """Create a new branch from the updated TEMPLATE branch This branch will then be used to create the PR """ log.info("Checking out merge base branch {}".format(self.merge_branch)) try: self.repo.create_head(self.merge_branch) except git.exc.GitCommandError: - raise SyncException("Could not checkout branch '{}'".format(self.merge_branch)) + raise SyncException("Could not create new branch '{}'".format(self.merge_branch)) def push_merge_branch(self): """Push the newly create merge branch to the remote repository""" @@ -370,73 +374,8 @@ def make_pull_request(self): "please see the [nf-core/tools v{tag} release page](https://github.com/nf-core/tools/releases/tag/{tag})." ).format(tag=nf_core.__version__) - # Try to update an existing pull-request - if self.update_existing_pull_request(pr_title, pr_body_text) is False: - # None found - make a new pull-request - self.submit_pull_request(pr_title, pr_body_text) - - def update_existing_pull_request(self, pr_title, pr_body_text): - """ - List existing pull-requests between TEMPLATE and self.from_branch - - If one is found, attempt to update it with a new title and body text - If none are found, return False - """ - assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" - # Look for existing pull-requests - list_prs_url = "https://api.github.com/repos/{}/pulls?head=nf-core:TEMPLATE&base={}".format( - self.gh_repo, self.from_branch - ) - r = requests.get( - url=list_prs_url, - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), - ) - try: - r_json = json.loads(r.content) - r_pp = json.dumps(r_json, indent=4) - except: - r_json = r.content - r_pp = r.content - - # PR worked - if r.status_code == 200: - log.debug("GitHub API listing existing PRs:\n{}".format(r_pp)) - - # No open PRs - if len(r_json) == 0: - log.info("No open PRs found between TEMPLATE and {}".format(self.from_branch)) - return False - - # Update existing PR - pr_update_api_url = r_json[0]["url"] - pr_content = {"title": pr_title, "body": pr_body_text} - - r = requests.patch( - url=pr_update_api_url, - data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), - ) - try: - r_json = json.loads(r.content) - r_pp = json.dumps(r_json, indent=4) - except: - r_json = r.content - r_pp = r.content - - # PR update worked - if r.status_code == 200: - log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) - log.info("Updated GitHub PR: {}".format(r_json["html_url"])) - return True - # Something went wrong - else: - log.warning("Could not update PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp)) - return False - - # Something went wrong - else: - log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) - return False + # Make new pull-request + self.submit_pull_request(pr_title, pr_body_text) def submit_pull_request(self, pr_title, pr_body_text): """ From ee32afb256ce8bba2eb8d4c2a66db365f7e2b3fb Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 8 Jan 2021 12:15:49 +0100 Subject: [PATCH 07/36] typo --- nf_core/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 2eb0682ba2..756ac8987a 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -328,7 +328,7 @@ def create_merge_base_branch(self): raise SyncException("Could not create new branch '{}'".format(self.merge_branch)) def push_merge_branch(self): - """Push the newly create merge branch to the remote repository""" + """Push the newly created merge branch to the remote repository""" log.info("Pushing {} branch to remote".format(self.merge_branch)) try: origin = self.repo.remote() From 563b836fddb8999fe127e620b48a20b1eda8ff30 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 8 Jan 2021 14:26:50 +0100 Subject: [PATCH 08/36] raise error if branch exists --- nf_core/sync.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 756ac8987a..c424e9f486 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -251,8 +251,7 @@ def close_open_template_merge_pull_requests(self): assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" log.info("Checking for open PRs from template merge branches") # Get list of all branches - branch_list = self.repo.branches - branch_list = [b.name for b in branch_list] + branch_list = [b.name for b in self.repo.branches] # Subset to template merging branches branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge-")] for branch in branch_list: @@ -321,6 +320,11 @@ def create_merge_base_branch(self): """Create a new branch from the updated TEMPLATE branch This branch will then be used to create the PR """ + # Check if branch exists already + branch_list = [b.name for b in self.repo.branches] + if self.merge_branch in branch_list: + raise SyncException("Branch already exists: '{}'".format(self.merge_branch)) + # Create new branch and checkout log.info("Checking out merge base branch {}".format(self.merge_branch)) try: self.repo.create_head(self.merge_branch) From cd94c1cb3b5b0e764d99ba9705689c5d9cfc38d0 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Tue, 12 Jan 2021 08:25:16 +0100 Subject: [PATCH 09/36] Create numbered new branch if already existing --- nf_core/sync.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index c424e9f486..26ef7e4710 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -323,7 +323,19 @@ def create_merge_base_branch(self): # Check if branch exists already branch_list = [b.name for b in self.repo.branches] if self.merge_branch in branch_list: - raise SyncException("Branch already exists: '{}'".format(self.merge_branch)) + original_merge_branch = self.merge_branch + # Try to create new branch with number at the end + # If -2 already exists, increase the number until branch is new + branch_no = 1 + while self.merge_branch in branch_list: + branch_no += 1 + self.merge_branch = self.merge_branch + "-" + str(branch_no) + log.info( + "Branch already existed: '{}', creating branch '{}' instead.".format( + original_merge_branch, self.merge_branch + ) + ) + # Create new branch and checkout log.info("Checking out merge base branch {}".format(self.merge_branch)) try: From b6510af95ba9c67363b8bf6bfd7af1cb2bc92f73 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Tue, 12 Jan 2021 08:28:28 +0100 Subject: [PATCH 10/36] fixed automated branch naming --- nf_core/sync.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 26ef7e4710..469a1ec8b5 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -326,10 +326,11 @@ def create_merge_base_branch(self): original_merge_branch = self.merge_branch # Try to create new branch with number at the end # If -2 already exists, increase the number until branch is new - branch_no = 1 + branch_no = 2 + self.merge_branch = original_merge_branch + "-" + str(branch_no) while self.merge_branch in branch_list: branch_no += 1 - self.merge_branch = self.merge_branch + "-" + str(branch_no) + self.merge_branch = original_merge_branch + "-" + str(branch_no) log.info( "Branch already existed: '{}', creating branch '{}' instead.".format( original_merge_branch, self.merge_branch From e19c324dc02ed940f2ac6c6f72babef29d50c8e3 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Tue, 12 Jan 2021 08:49:02 +0100 Subject: [PATCH 11/36] add comment to closed PRs --- nf_core/sync.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 469a1ec8b5..bf38d07581 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -67,7 +67,7 @@ def __init__( self.pipeline_dir = os.path.abspath(pipeline_dir) self.from_branch = from_branch self.original_branch = None - self.merge_branch = "nf-core-template-merge-11{}".format(nf_core.__version__) + self.merge_branch = "nf-core-template-merge-{}".format(nf_core.__version__) self.made_changes = False self.make_pr = make_pr self.gh_pr_returned_data = {} @@ -287,8 +287,16 @@ def close_open_pr(self, branch): return False # Close existing PR + pr_title = "Important! Template update for nf-core/tools v{} Closed because outdated!".format( + nf_core.__version__ + ) + pr_body_text = ( + "A new release of the main template in nf-core/tools has just been released. " + "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" + "This pull-request is outdated and has been closed. A new pull-request has been created instead." + ) pr_update_api_url = r_json[0]["url"] - pr_content = {"state": "closed"} + pr_content = {"state": "closed", "title": pr_title, "body": pr_body_text} r = requests.patch( url=pr_update_api_url, From ad323dc7900b3ff213cd109259f41199b498c5e3 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Tue, 12 Jan 2021 08:57:59 +0100 Subject: [PATCH 12/36] fixed typos --- nf_core/sync.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index bf38d07581..5b63bcf3e6 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -244,9 +244,9 @@ def push_template_branch(self): raise PullRequestException("Could not push TEMPLATE branch:\n {}".format(e)) def close_open_template_merge_pull_requests(self): - """Get all template merging branches (start with 'nf-core-template-merge-') - and check of any open PRs from these branches to the self.from_branch - If open PRs are found, close them + """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 """ assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" log.info("Checking for open PRs from template merge branches") From 04ed29854990dd7f43556858a16766368d0c9087 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Tue, 12 Jan 2021 09:48:17 +0100 Subject: [PATCH 13/36] update sync tests --- tests/test_sync.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_sync.py b/tests/test_sync.py index c7726cfb7d..2390ebbd72 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -191,7 +191,7 @@ def __init__(self, data, status_code): self.status_code = status_code self.content = json.dumps(data) - url_template = "https://api.github.com/repos/{}/response/pulls?head=nf-core:TEMPLATE&base=None" + url_template = "https://api.github.com/repos/{}/response/pulls?head=TEMPLATE&base=None" if kwargs["url"] == url_template.format("no_existing_pr"): response_data = [] return MockResponse(response_data, 200) @@ -258,9 +258,9 @@ def test_make_pull_request_bad_response(self, mock_get, mock_post): @mock.patch("requests.get", side_effect=mocked_requests_get) @mock.patch("requests.patch", side_effect=mocked_requests_patch) def test_update_existing_pull_request(self, mock_get, mock_patch): - """ Try discovering a PR and updating it """ + """ Try closing a PR """ psync = nf_core.sync.PipelineSync(self.pipeline_dir) psync.gh_username = "existing_pr" psync.gh_repo = "existing_pr/response" os.environ["GITHUB_AUTH_TOKEN"] = "test" - assert psync.update_existing_pull_request("title", "body") is True + assert psync.close_open_pr("TEMPLATE") is True From 532b255752b2c9e9718334342452f4df231e8d44 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 18 Jan 2021 08:56:21 +0100 Subject: [PATCH 14/36] added github auth token to create-lint-wf action --- .github/workflows/create-lint-wf.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index f9656a2562..48793ddb48 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -28,6 +28,8 @@ jobs: sudo ln -s /tmp/nextflow/nextflow /usr/local/bin/nextflow - name: Run nf-core/tools + env: + GITHUB_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | nf-core --log-file log.txt create -n testpipeline -d "This pipeline is for testing" -a "Testing McTestface" nf-core --log-file log.txt lint nf-core-testpipeline From ec3885bfeda5ca9bb43071f4003e17ed632bc010 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 1 Feb 2021 16:09:34 +0100 Subject: [PATCH 15/36] testing different token --- .github/workflows/create-lint-wf.yml | 2 +- nf_core/sync.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index 48793ddb48..02caf57ca5 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -29,7 +29,7 @@ jobs: - name: Run nf-core/tools env: - GITHUB_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_AUTH_TOKEN: ${{ secrets.nf_core_bot_auth_token }} run: | nf-core --log-file log.txt create -n testpipeline -d "This pipeline is for testing" -a "Testing McTestface" nf-core --log-file log.txt lint nf-core-testpipeline diff --git a/nf_core/sync.py b/nf_core/sync.py index 5b63bcf3e6..8ce5736138 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -254,8 +254,8 @@ def close_open_template_merge_pull_requests(self): branch_list = [b.name for b in self.repo.branches] # Subset to template merging branches branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge-")] + # Check for open PRs and close if found for branch in branch_list: - # Check for open PRs and close if found self.close_open_pr(branch) def close_open_pr(self, branch): From e399f8b77ce14fcb53c92ee6a1aad3479eb0ca3e Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 1 Feb 2021 16:14:29 +0100 Subject: [PATCH 16/36] revert token change --- .github/workflows/create-lint-wf.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index 02caf57ca5..48793ddb48 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -29,7 +29,7 @@ jobs: - name: Run nf-core/tools env: - GITHUB_AUTH_TOKEN: ${{ secrets.nf_core_bot_auth_token }} + GITHUB_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | nf-core --log-file log.txt create -n testpipeline -d "This pipeline is for testing" -a "Testing McTestface" nf-core --log-file log.txt lint nf-core-testpipeline From 61644ab2ed2ed4a70e2775de0fa4681dd47571dc Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 1 Feb 2021 16:20:52 +0100 Subject: [PATCH 17/36] test again with nf-core bot token --- .github/workflows/create-lint-wf.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index 48793ddb48..02caf57ca5 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -29,7 +29,7 @@ jobs: - name: Run nf-core/tools env: - GITHUB_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_AUTH_TOKEN: ${{ secrets.nf_core_bot_auth_token }} run: | nf-core --log-file log.txt create -n testpipeline -d "This pipeline is for testing" -a "Testing McTestface" nf-core --log-file log.txt lint nf-core-testpipeline From ef970fe1a835c90bf6c99887d1a0a1555fba7e39 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 8 Feb 2021 15:02:36 +0100 Subject: [PATCH 18/36] removed assert statement --- .github/workflows/sync.yml | 2 +- nf_core/sync.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/sync.yml b/.github/workflows/sync.yml index 6d5bd24c6b..e43ffcefb0 100644 --- a/.github/workflows/sync.yml +++ b/.github/workflows/sync.yml @@ -30,7 +30,7 @@ jobs: with: repository: nf-core/${{ matrix.pipeline }} ref: dev - token: ${{ secrets.nf_core_bot_auth_token }} + token: ${{ secrets.GITHUB_TOKEN }} path: nf-core/${{ matrix.pipeline }} fetch-depth: "0" diff --git a/nf_core/sync.py b/nf_core/sync.py index 8ce5736138..1d2e391000 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -375,9 +375,7 @@ def make_pull_request(self): raise PullRequestException("Could not find GitHub username and repo name") # If we've been asked to make a PR, check that we have the credentials - try: - assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" - except AssertionError: + if os.environ.get("GITHUB_AUTH_TOKEN", "") == "": raise PullRequestException( "Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR\n" "Make a PR at the following URL:\n https://github.com/{}/compare/{}...TEMPLATE".format( From ad3b277eaeb4eb56c142625649d960e2b3d07913 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 8 Feb 2021 15:07:33 +0100 Subject: [PATCH 19/36] removed second assert statement --- nf_core/sync.py | 58 +++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 1d2e391000..6985a6663a 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -404,37 +404,39 @@ def submit_pull_request(self, pr_title, pr_body_text): """ Create a new pull-request on GitHub """ - assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" - pr_content = { - "title": pr_title, - "body": pr_body_text, - "maintainer_can_modify": True, - "head": self.merge_branch, - "base": self.from_branch, - } - - r = requests.post( - url="https://api.github.com/repos/{}/pulls".format(self.gh_repo), - data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), - ) - try: - self.gh_pr_returned_data = json.loads(r.content) - returned_data_prettyprint = json.dumps(self.gh_pr_returned_data, indent=4) - except: - self.gh_pr_returned_data = r.content - returned_data_prettyprint = r.content + if not os.environ.get("GITHUB_AUTH_TOKEN", "") == "": + pr_content = { + "title": pr_title, + "body": pr_body_text, + "maintainer_can_modify": True, + "head": self.merge_branch, + "base": self.from_branch, + } + + r = requests.post( + url="https://api.github.com/repos/{}/pulls".format(self.gh_repo), + data=json.dumps(pr_content), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + ) + try: + self.gh_pr_returned_data = json.loads(r.content) + returned_data_prettyprint = json.dumps(self.gh_pr_returned_data, indent=4) + except: + self.gh_pr_returned_data = r.content + returned_data_prettyprint = r.content - # PR worked - if r.status_code == 201: - log.debug("GitHub API PR worked:\n{}".format(returned_data_prettyprint)) - log.info("GitHub PR created: {}".format(self.gh_pr_returned_data["html_url"])) + # PR worked + if r.status_code == 201: + log.debug("GitHub API PR worked:\n{}".format(returned_data_prettyprint)) + log.info("GitHub PR created: {}".format(self.gh_pr_returned_data["html_url"])) - # Something went wrong + # Something went wrong + else: + raise PullRequestException( + "GitHub API returned code {}: \n{}".format(r.status_code, returned_data_prettyprint) + ) else: - raise PullRequestException( - "GitHub API returned code {}: \n{}".format(r.status_code, returned_data_prettyprint) - ) + raise PullRequestException("Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR") def reset_target_dir(self): """ From 5645ef2c465ec6dc29d9c1a251a01d95dea7d1ce Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 8 Feb 2021 15:13:59 +0100 Subject: [PATCH 20/36] testing without GITHUB_TOKEN --- .github/workflows/sync.yml | 2 +- nf_core/sync.py | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.github/workflows/sync.yml b/.github/workflows/sync.yml index e43ffcefb0..6d5bd24c6b 100644 --- a/.github/workflows/sync.yml +++ b/.github/workflows/sync.yml @@ -30,7 +30,7 @@ jobs: with: repository: nf-core/${{ matrix.pipeline }} ref: dev - token: ${{ secrets.GITHUB_TOKEN }} + token: ${{ secrets.nf_core_bot_auth_token }} path: nf-core/${{ matrix.pipeline }} fetch-depth: "0" diff --git a/nf_core/sync.py b/nf_core/sync.py index 6985a6663a..9ccf715bba 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -249,14 +249,17 @@ def close_open_template_merge_pull_requests(self): If open PRs are found, add a comment and close them """ assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" - log.info("Checking for open PRs from template merge branches") - # Get list of all branches - branch_list = [b.name for b in self.repo.branches] - # Subset to template merging branches - branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge-")] - # Check for open PRs and close if found - for branch in branch_list: - self.close_open_pr(branch) + try: + log.info("Checking for open PRs from template merge branches") + # Get list of all branches + branch_list = [b.name for b in self.repo.branches] + # Subset to template merging branches + branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge-")] + # Check for open PRs and close if found + for branch in branch_list: + self.close_open_pr(branch) + except Exception as e: + raise PullRequestException("Could not close open pull requests! {}".format(e)) def close_open_pr(self, branch): """Given a branch, check for open PRs from that branch to self.from_branch From d60d7e29b177b69d4298f5c63efd9e3ad1cda8d1 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 8 Feb 2021 15:16:19 +0100 Subject: [PATCH 21/36] bufix --- nf_core/sync.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 9ccf715bba..57abadb32b 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -248,7 +248,6 @@ def close_open_template_merge_pull_requests(self): 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 """ - assert os.environ.get("GITHUB_AUTH_TOKEN", "") != "" try: log.info("Checking for open PRs from template merge branches") # Get list of all branches From 53ecbcc0533db04ade2ab77f96d28a86d7d60114 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 8 Feb 2021 15:27:22 +0100 Subject: [PATCH 22/36] removed remaining assert statements --- nf_core/sync.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 57abadb32b..b012e075d9 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -370,10 +370,7 @@ def make_pull_request(self): Returns: An instance of class requests.Response """ # Check that we know the github username and repo name - try: - assert self.gh_username is not None - assert self.gh_repo is not None - except AssertionError: + if self.gh_username is None and self.gh_repo is None: raise PullRequestException("Could not find GitHub username and repo name") # If we've been asked to make a PR, check that we have the credentials From e2cb5b25275c8bc1b6c89a8e17752cb320c023d2 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 8 Feb 2021 15:37:11 +0100 Subject: [PATCH 23/36] switched exception to log.error --- nf_core/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index b012e075d9..fca594088b 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -258,7 +258,7 @@ def close_open_template_merge_pull_requests(self): for branch in branch_list: self.close_open_pr(branch) except Exception as e: - raise PullRequestException("Could not close open pull requests! {}".format(e)) + raise log.error("Could not close open pull requests! {}".format(e)) def close_open_pr(self, branch): """Given a branch, check for open PRs from that branch to self.from_branch From d3a748f66f6977aae1770e20619c09acec26de3c Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Thu, 11 Feb 2021 08:01:45 +0100 Subject: [PATCH 24/36] updated changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f47624796e..e5c34ade3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,9 @@ ## v1.13dev -_..nothing yet.._ +### Tools helper code + +* changed behaviour of `nf-core sync` command as discussed in [[#787]](https://github.com/nf-core/tools/issues/787) ## [v1.12.1 - Silver Dolphin](https://github.com/nf-core/tools/releases/tag/1.12.1) - [2020-12-03] From 547d5627bcd7f0a871616637518e3683dcf7a1f6 Mon Sep 17 00:00:00 2001 From: Kevin Menden Date: Fri, 12 Feb 2021 13:33:56 +0100 Subject: [PATCH 25/36] Apply suggestions from code review Co-authored-by: Phil Ewels --- CHANGELOG.md | 4 +++- nf_core/sync.py | 21 ++++++++------------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97eefc008c..485a39aa22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,9 @@ * Singularity images in module files are now discovered and fetched * Direct downloads of Singularity images in python allowed (much faster than running `singularity pull`) * Downloads now work with `$NXF_SINGULARITY_CACHEDIR` so that pipelines sharing containers have efficient downloads -* changed behaviour of `nf-core sync` command as discussed in [[#787]](https://github.com/nf-core/tools/issues/787) +* Changed behaviour of `nf-core sync` command [[#787](https://github.com/nf-core/tools/issues/787)] + * Instead of opening or updating a PR from `TEMPLATE` directly to `dev`, a new branch is now created from `TEMPLATE` and a PR opened from this to `dev`. + * This is to make it easier to fix merge conflicts without accidentally bringing the entire pipeline history back into the `TEMPLATE` branch (which makes subsequent sync merges much more difficult) ### Linting diff --git a/nf_core/sync.py b/nf_core/sync.py index fca594088b..ffcc5cd913 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -250,15 +250,12 @@ def close_open_template_merge_pull_requests(self): """ try: log.info("Checking for open PRs from template merge branches") - # Get list of all branches - branch_list = [b.name for b in self.repo.branches] - # Subset to template merging branches - branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge-")] # Check for open PRs and close if found - for branch in branch_list: + for branch in [b.name for b in self.repo.branches if b.name.startswith("nf-core-template-merge-")]: self.close_open_pr(branch) except Exception as e: - raise log.error("Could not close open pull requests! {}".format(e)) + log.error("Could not close open pull requests! {}".format(e)) + raise def close_open_pr(self, branch): """Given a branch, check for open PRs from that branch to self.from_branch @@ -266,9 +263,7 @@ def close_open_pr(self, branch): """ log.info("Checking branch: {}".format(branch)) # Look for existing pull-requests - list_prs_url = "https://api.github.com/repos/{}/pulls?head={}&base={}".format( - self.gh_repo, branch, self.from_branch - ) + list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls?head={branch}&base={self.from_branch}" r = requests.get( url=list_prs_url, auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), @@ -319,7 +314,7 @@ def close_open_pr(self, branch): return True # Something went wrong else: - log.warning("Could not close PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp)) + log.warning(f"Could not close PR ('{r.status_code}'):\n{pr_update_api_url}\n{r_pp}") return False # Something went wrong @@ -351,8 +346,8 @@ def create_merge_base_branch(self): log.info("Checking out merge base branch {}".format(self.merge_branch)) try: self.repo.create_head(self.merge_branch) - except git.exc.GitCommandError: - raise SyncException("Could not create new branch '{}'".format(self.merge_branch)) + except git.exc.GitCommandError as e: + raise SyncException(f"Could not create new branch '{self.merge_branch}'\n{e}") def push_merge_branch(self): """Push the newly created merge branch to the remote repository""" @@ -361,7 +356,7 @@ def push_merge_branch(self): origin = self.repo.remote() origin.push(self.merge_branch) except git.exc.GitCommandError as e: - raise PullRequestException("Could not push {} branch:\n {}".format(self.merge_branch, e)) + raise PullRequestException(f"Could not push branch '{self.merge_branch}':\n {e}") def make_pull_request(self): """Create a pull request to a base branch (default: dev), From 6c912aec7ca6d643ded51e605b1f1169614322fe Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 12 Feb 2021 14:04:48 +0100 Subject: [PATCH 26/36] check for token in sync --- nf_core/sync.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index ffcc5cd913..f480f2263b 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -85,6 +85,9 @@ def sync(self): if self.make_pr: log.info("Will attempt to automatically create a pull request") + if os.environ.get("GITHUB_AUTH_TOKEN", "") == "": + raise SyncException("GITHUB_AUTH_TOKEN not set!") + self.inspect_sync_dir() self.get_wf_config() self.close_open_template_merge_pull_requests() @@ -248,14 +251,10 @@ def close_open_template_merge_pull_requests(self): 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 """ - try: - log.info("Checking for open PRs from template merge branches") - # Check for open PRs and close if found - for branch in [b.name for b in self.repo.branches if b.name.startswith("nf-core-template-merge-")]: - self.close_open_pr(branch) - except Exception as e: - log.error("Could not close open pull requests! {}".format(e)) - raise + log.info("Checking for open PRs from template merge branches") + # Check for open PRs and close if found + for branch in [b.name for b in self.repo.branches if b.name.startswith("nf-core-template-merge-")]: + self.close_open_pr(branch) def close_open_pr(self, branch): """Given a branch, check for open PRs from that branch to self.from_branch From 4681a6d825b893a933a13fe831492d7c60d678f8 Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 12 Feb 2021 15:04:53 +0100 Subject: [PATCH 27/36] moved token checking code --- nf_core/sync.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index f480f2263b..43aeeb092b 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -85,12 +85,8 @@ def sync(self): if self.make_pr: log.info("Will attempt to automatically create a pull request") - if os.environ.get("GITHUB_AUTH_TOKEN", "") == "": - raise SyncException("GITHUB_AUTH_TOKEN not set!") - self.inspect_sync_dir() self.get_wf_config() - self.close_open_template_merge_pull_requests() self.checkout_template_branch() self.delete_template_branch_files() self.make_template_pipeline() @@ -99,8 +95,11 @@ def sync(self): # Push and make a pull request if we've been asked to if self.made_changes and self.make_pr: try: + if os.environ.get("GITHUB_AUTH_TOKEN", "") == "": + raise PullRequestException("GITHUB_AUTH_TOKEN not set!") self.push_template_branch() self.create_merge_base_branch() + self.close_open_template_merge_pull_requests() self.push_merge_branch() self.make_pull_request() except PullRequestException as e: From 8a288c3bac5fe9d27da288c8b8134e77ccfdf5cd Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Fri, 12 Feb 2021 16:27:17 +0100 Subject: [PATCH 28/36] commenting closed PR instead of changing it --- nf_core/sync.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 43aeeb092b..60f0633a96 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -75,6 +75,7 @@ def __init__( self.gh_username = gh_username self.gh_repo = gh_repo + self.pr_url = "" def sync(self): """Find workflow attributes, create a new template pipeline on TEMPLATE""" @@ -99,9 +100,9 @@ def sync(self): raise PullRequestException("GITHUB_AUTH_TOKEN not set!") self.push_template_branch() self.create_merge_base_branch() - self.close_open_template_merge_pull_requests() self.push_merge_branch() self.make_pull_request() + self.close_open_template_merge_pull_requests() except PullRequestException as e: self.reset_target_dir() raise PullRequestException(e) @@ -281,17 +282,23 @@ def close_open_pr(self, branch): log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) return False - # Close existing PR - pr_title = "Important! Template update for nf-core/tools v{} Closed because outdated!".format( - nf_core.__version__ - ) - pr_body_text = ( + # Make a new comment + comment_text = ( "A new release of the main template in nf-core/tools has just been released. " "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" "This pull-request is outdated and has been closed. A new pull-request has been created instead." + "Link to new PR: {}".format(self.pr_url) ) + comment_content = {"body": comment_text} + comments_url = r_json[0]["comments_url"] + comment_r = requests.post( + url=comments_url, + data=json.dumps(comment_content), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + ) + pr_update_api_url = r_json[0]["url"] - pr_content = {"state": "closed", "title": pr_title, "body": pr_body_text} + pr_content = {"state": "closed"} r = requests.patch( url=pr_update_api_url, @@ -419,6 +426,7 @@ def submit_pull_request(self, pr_title, pr_body_text): # PR worked if r.status_code == 201: + self.pr_url = self.gh_pr_returned_data["html_url"] log.debug("GitHub API PR worked:\n{}".format(returned_data_prettyprint)) log.info("GitHub PR created: {}".format(self.gh_pr_returned_data["html_url"])) From bc302eeb7f77638949002e01acd91613d277deed Mon Sep 17 00:00:00 2001 From: kevinmenden Date: Mon, 15 Feb 2021 09:15:28 +0100 Subject: [PATCH 29/36] don't close new PR --- nf_core/sync.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nf_core/sync.py b/nf_core/sync.py index 60f0633a96..2a95192f99 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -254,6 +254,9 @@ def close_open_template_merge_pull_requests(self): log.info("Checking for open PRs from template merge branches") # Check for open PRs and close if found for branch in [b.name for b in self.repo.branches if b.name.startswith("nf-core-template-merge-")]: + # Don't close the new merge branch + if branch == self.merge_branch: + continue self.close_open_pr(branch) def close_open_pr(self, branch): From 488f63b872136d09cb8348f5850d714ef6f76729 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 16 Feb 2021 20:46:42 +0100 Subject: [PATCH 30/36] Sync: Clean up simple stuff from review on PR See nf-core/tools#821 --- .github/workflows/create-lint-wf.yml | 2 - nf_core/sync.py | 70 +++++++++++----------------- 2 files changed, 28 insertions(+), 44 deletions(-) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index 02caf57ca5..f9656a2562 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -28,8 +28,6 @@ jobs: sudo ln -s /tmp/nextflow/nextflow /usr/local/bin/nextflow - name: Run nf-core/tools - env: - GITHUB_AUTH_TOKEN: ${{ secrets.nf_core_bot_auth_token }} run: | nf-core --log-file log.txt create -n testpipeline -d "This pipeline is for testing" -a "Testing McTestface" nf-core --log-file log.txt lint nf-core-testpipeline diff --git a/nf_core/sync.py b/nf_core/sync.py index 2a95192f99..a3723c2d91 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -340,10 +340,10 @@ def create_merge_base_branch(self): # Try to create new branch with number at the end # If -2 already exists, increase the number until branch is new branch_no = 2 - self.merge_branch = original_merge_branch + "-" + str(branch_no) + self.merge_branch = f"{original_merge_branch}-{branch_no}" while self.merge_branch in branch_list: branch_no += 1 - self.merge_branch = original_merge_branch + "-" + str(branch_no) + self.merge_branch = f"{original_merge_branch}-{branch_no}" log.info( "Branch already existed: '{}', creating branch '{}' instead.".format( original_merge_branch, self.merge_branch @@ -376,15 +376,6 @@ def make_pull_request(self): if self.gh_username is None and self.gh_repo is None: raise PullRequestException("Could not find GitHub username and repo name") - # If we've been asked to make a PR, check that we have the credentials - if os.environ.get("GITHUB_AUTH_TOKEN", "") == "": - raise PullRequestException( - "Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR\n" - "Make a PR at the following URL:\n https://github.com/{}/compare/{}...TEMPLATE".format( - self.gh_repo, self.original_branch - ) - ) - log.info("Submitting a pull request via the GitHub API") pr_title = "Important! Template update for nf-core/tools v{}".format(nf_core.__version__) @@ -406,40 +397,35 @@ def submit_pull_request(self, pr_title, pr_body_text): """ Create a new pull-request on GitHub """ - if not os.environ.get("GITHUB_AUTH_TOKEN", "") == "": - pr_content = { - "title": pr_title, - "body": pr_body_text, - "maintainer_can_modify": True, - "head": self.merge_branch, - "base": self.from_branch, - } - - r = requests.post( - url="https://api.github.com/repos/{}/pulls".format(self.gh_repo), - data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), - ) - try: - self.gh_pr_returned_data = json.loads(r.content) - returned_data_prettyprint = json.dumps(self.gh_pr_returned_data, indent=4) - except: - self.gh_pr_returned_data = r.content - returned_data_prettyprint = r.content + pr_content = { + "title": pr_title, + "body": pr_body_text, + "maintainer_can_modify": True, + "head": self.merge_branch, + "base": self.from_branch, + } + + r = requests.post( + url="https://api.github.com/repos/{}/pulls".format(self.gh_repo), + data=json.dumps(pr_content), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + ) + try: + self.gh_pr_returned_data = json.loads(r.content) + returned_data_prettyprint = json.dumps(self.gh_pr_returned_data, indent=4) + except: + self.gh_pr_returned_data = r.content + returned_data_prettyprint = r.content - # PR worked - if r.status_code == 201: - self.pr_url = self.gh_pr_returned_data["html_url"] - log.debug("GitHub API PR worked:\n{}".format(returned_data_prettyprint)) - log.info("GitHub PR created: {}".format(self.gh_pr_returned_data["html_url"])) + # PR worked + if r.status_code == 201: + self.pr_url = self.gh_pr_returned_data["html_url"] + log.debug("GitHub API PR worked:\n{}".format(returned_data_prettyprint)) + log.info("GitHub PR created: {}".format(self.gh_pr_returned_data["html_url"])) - # Something went wrong - else: - raise PullRequestException( - "GitHub API returned code {}: \n{}".format(r.status_code, returned_data_prettyprint) - ) + # Something went wrong else: - raise PullRequestException("Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR") + raise PullRequestException(f"GitHub API returned code {r.status_code}: \n{returned_data_prettyprint}") def reset_target_dir(self): """ From 8e1bdaf07b00840cec87ad14b90ca8d0c4c4f64e Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 16 Feb 2021 20:51:11 +0100 Subject: [PATCH 31/36] Minor tidying up for credentials checks, merge two small functions --- nf_core/sync.py | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index a3723c2d91..77eaf886f5 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -96,8 +96,14 @@ def sync(self): # Push and make a pull request if we've been asked to if self.made_changes and self.make_pr: try: + # Check that we have an API auth token if os.environ.get("GITHUB_AUTH_TOKEN", "") == "": raise PullRequestException("GITHUB_AUTH_TOKEN not set!") + + # Check that we know the github username and repo name + if self.gh_username is None and self.gh_repo is None: + raise PullRequestException("Could not find GitHub username and repo name") + self.push_template_branch() self.create_merge_base_branch() self.push_merge_branch() @@ -268,7 +274,7 @@ def close_open_pr(self, branch): list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls?head={branch}&base={self.from_branch}" r = requests.get( url=list_prs_url, - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), ) try: r_json = json.loads(r.content) @@ -297,7 +303,7 @@ def close_open_pr(self, branch): comment_r = requests.post( url=comments_url, data=json.dumps(comment_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), ) pr_update_api_url = r_json[0]["url"] @@ -306,7 +312,7 @@ def close_open_pr(self, branch): r = requests.patch( url=pr_update_api_url, data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), ) try: r_json = json.loads(r.content) @@ -372,13 +378,9 @@ def make_pull_request(self): Returns: An instance of class requests.Response """ - # Check that we know the github username and repo name - if self.gh_username is None and self.gh_repo is None: - raise PullRequestException("Could not find GitHub username and repo name") - log.info("Submitting a pull request via the GitHub API") - pr_title = "Important! Template update for nf-core/tools v{}".format(nf_core.__version__) + pr_title = f"Important! Template update for nf-core/tools v{nf_core.__version__}" pr_body_text = ( "A new release of the main template in nf-core/tools has just been released. " "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" @@ -391,12 +393,6 @@ def make_pull_request(self): ).format(tag=nf_core.__version__) # Make new pull-request - self.submit_pull_request(pr_title, pr_body_text) - - def submit_pull_request(self, pr_title, pr_body_text): - """ - Create a new pull-request on GitHub - """ pr_content = { "title": pr_title, "body": pr_body_text, @@ -408,7 +404,7 @@ def submit_pull_request(self, pr_title, pr_body_text): r = requests.post( url="https://api.github.com/repos/{}/pulls".format(self.gh_repo), data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), ) try: self.gh_pr_returned_data = json.loads(r.content) From 7d04fbd01c9fe0acd8bd9846942fc5432ace085f Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 16 Feb 2021 20:53:25 +0100 Subject: [PATCH 32/36] Reorder functions to match order of execution --- nf_core/sync.py | 166 ++++++++++++++++++++++++------------------------ 1 file changed, 83 insertions(+), 83 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 77eaf886f5..c79b607d40 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -252,89 +252,6 @@ def push_template_branch(self): except git.exc.GitCommandError as e: raise PullRequestException("Could not push TEMPLATE branch:\n {}".format(e)) - def close_open_template_merge_pull_requests(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") - # Check for open PRs and close if found - for branch in [b.name for b in self.repo.branches if b.name.startswith("nf-core-template-merge-")]: - # Don't close the new merge branch - if branch == self.merge_branch: - continue - self.close_open_pr(branch) - - def close_open_pr(self, branch): - """Given a branch, check for open PRs from that branch to self.from_branch - and close if PRs have been found - """ - log.info("Checking branch: {}".format(branch)) - # Look for existing pull-requests - list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls?head={branch}&base={self.from_branch}" - r = requests.get( - url=list_prs_url, - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) - try: - r_json = json.loads(r.content) - r_pp = json.dumps(r_json, indent=4) - except: - r_json = r.content - r_pp = r.content - - if r.status_code == 200: - log.debug("GitHub API listing existing PRs:\n{}".format(r_pp)) - - # No open PRs - if len(r_json) == 0: - log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) - return False - - # Make a new comment - comment_text = ( - "A new release of the main template in nf-core/tools has just been released. " - "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" - "This pull-request is outdated and has been closed. A new pull-request has been created instead." - "Link to new PR: {}".format(self.pr_url) - ) - comment_content = {"body": comment_text} - comments_url = r_json[0]["comments_url"] - comment_r = requests.post( - url=comments_url, - data=json.dumps(comment_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) - - pr_update_api_url = r_json[0]["url"] - pr_content = {"state": "closed"} - - r = requests.patch( - url=pr_update_api_url, - data=json.dumps(pr_content), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) - try: - r_json = json.loads(r.content) - r_pp = json.dumps(r_json, indent=4) - except: - r_json = r.content - r_pp = r.content - - # PR update worked - if r.status_code == 200: - log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) - log.info("Closed GitHub PR: {}".format(r_json["html_url"])) - return True - # Something went wrong - else: - log.warning(f"Could not close PR ('{r.status_code}'):\n{pr_update_api_url}\n{r_pp}") - return False - - # Something went wrong - else: - log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) - def create_merge_base_branch(self): """Create a new branch from the updated TEMPLATE branch This branch will then be used to create the PR @@ -423,6 +340,89 @@ def make_pull_request(self): else: raise PullRequestException(f"GitHub API returned code {r.status_code}: \n{returned_data_prettyprint}") + def close_open_template_merge_pull_requests(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") + # Check for open PRs and close if found + for branch in [b.name for b in self.repo.branches if b.name.startswith("nf-core-template-merge-")]: + # Don't close the new merge branch + if branch == self.merge_branch: + continue + self.close_open_pr(branch) + + def close_open_pr(self, branch): + """Given a branch, check for open PRs from that branch to self.from_branch + and close if PRs have been found + """ + log.info("Checking branch: {}".format(branch)) + # Look for existing pull-requests + list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls?head={branch}&base={self.from_branch}" + r = requests.get( + url=list_prs_url, + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), + ) + try: + r_json = json.loads(r.content) + r_pp = json.dumps(r_json, indent=4) + except: + r_json = r.content + r_pp = r.content + + if r.status_code == 200: + log.debug("GitHub API listing existing PRs:\n{}".format(r_pp)) + + # No open PRs + if len(r_json) == 0: + log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) + return False + + # Make a new comment + comment_text = ( + "A new release of the main template in nf-core/tools has just been released. " + "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" + "This pull-request is outdated and has been closed. A new pull-request has been created instead." + "Link to new PR: {}".format(self.pr_url) + ) + comment_content = {"body": comment_text} + comments_url = r_json[0]["comments_url"] + comment_r = requests.post( + url=comments_url, + data=json.dumps(comment_content), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), + ) + + pr_update_api_url = r_json[0]["url"] + pr_content = {"state": "closed"} + + r = requests.patch( + url=pr_update_api_url, + data=json.dumps(pr_content), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), + ) + try: + r_json = json.loads(r.content) + r_pp = json.dumps(r_json, indent=4) + except: + r_json = r.content + r_pp = r.content + + # PR update worked + if r.status_code == 200: + log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) + log.info("Closed GitHub PR: {}".format(r_json["html_url"])) + return True + # Something went wrong + else: + log.warning(f"Could not close PR ('{r.status_code}'):\n{pr_update_api_url}\n{r_pp}") + return False + + # Something went wrong + else: + log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) + def reset_target_dir(self): """ Reset the target pipeline directory. Check out the original branch. From 277b8676adadd87675b5cd7d2893040c53fc64c5 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 16 Feb 2021 21:04:40 +0100 Subject: [PATCH 33/36] Refactor close / comment code to avoid reusing variable names --- nf_core/sync.py | 68 ++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index c79b607d40..6674edbf42 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -346,12 +346,9 @@ def close_open_template_merge_pull_requests(self): If open PRs are found, add a comment and close them """ log.info("Checking for open PRs from template merge branches") - # Check for open PRs and close if found - for branch in [b.name for b in self.repo.branches if b.name.startswith("nf-core-template-merge-")]: - # Don't close the new merge branch - if branch == self.merge_branch: - continue - self.close_open_pr(branch) + for branch in [b.name for b in self.repo.branches]: + if branch.startswith("nf-core-template-merge-") and branch != self.merge_branch: + self.close_open_pr(branch) def close_open_pr(self, branch): """Given a branch, check for open PRs from that branch to self.from_branch @@ -360,68 +357,63 @@ def close_open_pr(self, branch): log.info("Checking branch: {}".format(branch)) # Look for existing pull-requests list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls?head={branch}&base={self.from_branch}" - r = requests.get( + list_prs_request = requests.get( url=list_prs_url, auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), ) try: - r_json = json.loads(r.content) - r_pp = json.dumps(r_json, indent=4) + list_prs_json = json.loads(list_prs_request.content) + list_prs_pp = json.dumps(list_prs_json, indent=4) except: - r_json = r.content - r_pp = r.content + list_prs_json = list_prs_request.content + list_prs_pp = list_prs_request.content - if r.status_code == 200: - log.debug("GitHub API listing existing PRs:\n{}".format(r_pp)) + if list_prs_request.status_code == 200: + log.debug("GitHub API listing existing PRs:\n{}".format(list_prs_pp)) # No open PRs - if len(r_json) == 0: + if len(list_prs_json) == 0: log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) return False - # Make a new comment + # Make a new comment explaining why the PR is being closed comment_text = ( - "A new release of the main template in nf-core/tools has just been released. " - "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" - "This pull-request is outdated and has been closed. A new pull-request has been created instead." - "Link to new PR: {}".format(self.pr_url) + f"Version {nf_core.__version__} of the @nf-core template in nf-core/tools has just been released.\n\n" + f"This pull-request is now outdated and has been closed in favour of {self.pr_url}" ) - comment_content = {"body": comment_text} - comments_url = r_json[0]["comments_url"] - comment_r = requests.post( - url=comments_url, - data=json.dumps(comment_content), + comment_request = requests.post( + url=list_prs_json[0]["comments_url"], + data=json.dumps({"body": comment_text}), auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), ) - pr_update_api_url = r_json[0]["url"] - pr_content = {"state": "closed"} - - r = requests.patch( + # Update the PR status to be closed + pr_update_api_url = list_prs_json[0]["url"] + pr_request = requests.patch( url=pr_update_api_url, - data=json.dumps(pr_content), + data=json.dumps({"state": "closed"}), auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), ) try: - r_json = json.loads(r.content) - r_pp = json.dumps(r_json, indent=4) + pr_request_json = json.loads(pr_request.content) + pr_request_pp = json.dumps(pr_request_json, indent=4) except: - r_json = r.content - r_pp = r.content + pr_request_json = pr_request.content + pr_request_pp = pr_request.content # PR update worked - if r.status_code == 200: - log.debug("GitHub API PR-update worked:\n{}".format(r_pp)) - log.info("Closed GitHub PR: {}".format(r_json["html_url"])) + if pr_request.status_code == 200: + log.debug("GitHub API PR-update worked:\n{}".format(pr_request_pp)) + log.info("Closed GitHub PR: {}".format(pr_request_json["html_url"])) return True # Something went wrong else: - log.warning(f"Could not close PR ('{r.status_code}'):\n{pr_update_api_url}\n{r_pp}") + log.warning(f"Could not close PR ('{pr_request.status_code}'):\n{pr_update_api_url}\n{pr_request_pp}") return False # Something went wrong else: - log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp)) + log.warning(f"Could not list open PRs ('{list_prs_request.status_code}')\n{list_prs_url}\n{list_prs_pp}") def reset_target_dir(self): """ From 94106e5ce4961b26e5a25f388b1296c7b459267d Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 16 Feb 2021 21:15:47 +0100 Subject: [PATCH 34/36] Deleted outdated sync pytests These tests are getting to be more effort to write than the code they're testing. Mocking up an entire functional GitHub API response for multiple calls is a little too much.. --- nf_core/sync.py | 4 ++-- tests/test_sync.py | 41 ----------------------------------------- 2 files changed, 2 insertions(+), 43 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 6674edbf42..82978d7fa8 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -108,7 +108,7 @@ def sync(self): self.create_merge_base_branch() self.push_merge_branch() self.make_pull_request() - self.close_open_template_merge_pull_requests() + self.close_open_template_merge_prs() except PullRequestException as e: self.reset_target_dir() raise PullRequestException(e) @@ -340,7 +340,7 @@ def make_pull_request(self): else: raise PullRequestException(f"GitHub API returned code {r.status_code}: \n{returned_data_prettyprint}") - def close_open_template_merge_pull_requests(self): + 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 diff --git a/tests/test_sync.py b/tests/test_sync.py index 2390ebbd72..4a34e68a0e 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -156,33 +156,6 @@ def test_push_template_branch_error(self): except nf_core.sync.PullRequestException as e: assert e.args[0].startswith("Could not push TEMPLATE branch") - def test_make_pull_request_missing_username(self): - """ Try making a PR without a repo or username """ - psync = nf_core.sync.PipelineSync(self.pipeline_dir) - psync.gh_username = None - psync.gh_repo = None - try: - psync.make_pull_request() - raise UserWarning("Should have hit an exception") - except nf_core.sync.PullRequestException as e: - assert e.args[0] == "Could not find GitHub username and repo name" - - def test_make_pull_request_missing_auth(self): - """ Try making a PR without any auth """ - psync = nf_core.sync.PipelineSync(self.pipeline_dir) - psync.gh_username = "foo" - psync.gh_repo = "foo/bar" - if "GITHUB_AUTH_TOKEN" in os.environ: - del os.environ["GITHUB_AUTH_TOKEN"] - try: - psync.make_pull_request() - raise UserWarning("Should have hit an exception") - except nf_core.sync.PullRequestException as e: - assert e.args[0] == ( - "Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR\n" - "Make a PR at the following URL:\n https://github.com/foo/bar/compare/None...TEMPLATE" - ) - def mocked_requests_get(**kwargs): """ Helper function to emulate POST requests responses from the web """ @@ -196,10 +169,6 @@ def __init__(self, data, status_code): response_data = [] return MockResponse(response_data, 200) - if kwargs["url"] == url_template.format("existing_pr"): - response_data = [{"url": "url_to_update_pr"}] - return MockResponse(response_data, 200) - return MockResponse({"get_url": kwargs["url"]}, 404) def mocked_requests_patch(**kwargs): @@ -254,13 +223,3 @@ def test_make_pull_request_bad_response(self, mock_get, mock_post): raise UserWarning("Should have hit an exception") except nf_core.sync.PullRequestException as e: assert e.args[0].startswith("GitHub API returned code 404:") - - @mock.patch("requests.get", side_effect=mocked_requests_get) - @mock.patch("requests.patch", side_effect=mocked_requests_patch) - def test_update_existing_pull_request(self, mock_get, mock_patch): - """ Try closing a PR """ - psync = nf_core.sync.PipelineSync(self.pipeline_dir) - psync.gh_username = "existing_pr" - psync.gh_repo = "existing_pr/response" - os.environ["GITHUB_AUTH_TOKEN"] = "test" - assert psync.close_open_pr("TEMPLATE") is True From 4b66b65c3257e8aedb43169161ac31300f275e74 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 16 Feb 2021 21:36:44 +0100 Subject: [PATCH 35/36] Minor tweaks to PR body text --- nf_core/sync.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 82978d7fa8..70e66563b8 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -299,15 +299,16 @@ def make_pull_request(self): pr_title = f"Important! Template update for nf-core/tools v{nf_core.__version__}" pr_body_text = ( - "A new release of the main template in nf-core/tools has just been released. " + "Version `{tag}` of [nf-core/tools](https://github.com/nf-core/tools) has just been released with updates to the nf-core template. " "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" - "Please make sure to merge this pull-request as soon as possible. " + "Please make sure to merge this pull-request as soon as possible, " + "resolving any merge conflicts in the `{merge_branch}` branch (or your own fork, if you prefer). " "Once complete, make a new minor release of your pipeline. " "For instructions on how to merge this PR, please see " "[https://nf-co.re/developers/sync](https://nf-co.re/developers/sync#merging-automated-prs).\n\n" "For more information about this release of [nf-core/tools](https://github.com/nf-core/tools), " - "please see the [nf-core/tools v{tag} release page](https://github.com/nf-core/tools/releases/tag/{tag})." - ).format(tag=nf_core.__version__) + "please see the `v{tag}` [release page](https://github.com/nf-core/tools/releases/tag/{tag})." + ).format(tag=nf_core.__version__, mb=self.merge_branch) # Make new pull-request pr_content = { From dc2148fd8a6b9e481c813f5489a5212ca7f9d74e Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 16 Feb 2021 23:09:11 +0100 Subject: [PATCH 36/36] Fix / refactor code that closes open PRs --- nf_core/sync.py | 124 ++++++++++++++++++++++++++---------------------- 1 file changed, 67 insertions(+), 57 deletions(-) diff --git a/nf_core/sync.py b/nf_core/sync.py index 70e66563b8..3c52b92067 100644 --- a/nf_core/sync.py +++ b/nf_core/sync.py @@ -9,6 +9,7 @@ import os import re import requests +import requests_cache import shutil import tempfile @@ -80,9 +81,12 @@ def __init__( def sync(self): """Find workflow attributes, create a new template pipeline on TEMPLATE""" + # Clear requests_cache so that we don't get stale API responses + requests_cache.clear() + log.info("Pipeline directory: {}".format(self.pipeline_dir)) if self.from_branch: - log.info("Using branch `{}` to fetch workflow variables".format(self.from_branch)) + log.info("Using branch '{}' to fetch workflow variables".format(self.from_branch)) if self.make_pr: log.info("Will attempt to automatically create a pull request") @@ -192,7 +196,7 @@ def delete_template_branch_files(self): Delete all files in the TEMPLATE branch """ # Delete everything - log.info("Deleting all files in TEMPLATE branch") + log.info("Deleting all files in 'TEMPLATE' branch") for the_file in os.listdir(self.pipeline_dir): if the_file == ".git": continue @@ -236,7 +240,7 @@ def commit_template_changes(self): self.repo.git.add(A=True) self.repo.index.commit("Template update for nf-core/tools version {}".format(nf_core.__version__)) self.made_changes = True - log.info("Committed changes to TEMPLATE branch") + log.info("Committed changes to 'TEMPLATE' branch") except Exception as e: raise SyncException("Could not commit changes to TEMPLATE:\n{}".format(e)) return True @@ -274,7 +278,7 @@ def create_merge_base_branch(self): ) # Create new branch and checkout - log.info("Checking out merge base branch {}".format(self.merge_branch)) + log.info(f"Checking out merge base branch '{self.merge_branch}'") try: self.repo.create_head(self.merge_branch) except git.exc.GitCommandError as e: @@ -282,7 +286,7 @@ def create_merge_base_branch(self): def push_merge_branch(self): """Push the newly created merge branch to the remote repository""" - log.info("Pushing {} branch to remote".format(self.merge_branch)) + log.info(f"Pushing '{self.merge_branch}' branch to remote") try: origin = self.repo.remote() origin.push(self.merge_branch) @@ -302,13 +306,13 @@ def make_pull_request(self): "Version `{tag}` of [nf-core/tools](https://github.com/nf-core/tools) has just been released with updates to the nf-core template. " "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n" "Please make sure to merge this pull-request as soon as possible, " - "resolving any merge conflicts in the `{merge_branch}` branch (or your own fork, if you prefer). " - "Once complete, make a new minor release of your pipeline. " + f"resolving any merge conflicts in the `{self.merge_branch}` branch (or your own fork, if you prefer). " + "Once complete, make a new minor release of your pipeline.\n\n" "For instructions on how to merge this PR, please see " "[https://nf-co.re/developers/sync](https://nf-co.re/developers/sync#merging-automated-prs).\n\n" "For more information about this release of [nf-core/tools](https://github.com/nf-core/tools), " "please see the `v{tag}` [release page](https://github.com/nf-core/tools/releases/tag/{tag})." - ).format(tag=nf_core.__version__, mb=self.merge_branch) + ).format(tag=nf_core.__version__) # Make new pull-request pr_content = { @@ -347,17 +351,9 @@ def close_open_template_merge_prs(self): If open PRs are found, add a comment and close them """ log.info("Checking for open PRs from template merge branches") - for branch in [b.name for b in self.repo.branches]: - if branch.startswith("nf-core-template-merge-") and branch != self.merge_branch: - self.close_open_pr(branch) - def close_open_pr(self, branch): - """Given a branch, check for open PRs from that branch to self.from_branch - and close if PRs have been found - """ - log.info("Checking branch: {}".format(branch)) # Look for existing pull-requests - list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls?head={branch}&base={self.from_branch}" + list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls" list_prs_request = requests.get( url=list_prs_url, auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), @@ -369,52 +365,66 @@ def close_open_pr(self, branch): list_prs_json = list_prs_request.content list_prs_pp = list_prs_request.content - if list_prs_request.status_code == 200: - log.debug("GitHub API listing existing PRs:\n{}".format(list_prs_pp)) + 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 - # No open PRs - if len(list_prs_json) == 0: - log.info("No open PRs found between {} and {}".format(branch, self.from_branch)) - return False + for pr in list_prs_json: + 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 - # Make a new comment explaining why the PR is being closed - comment_text = ( - f"Version {nf_core.__version__} of the @nf-core template in nf-core/tools has just been released.\n\n" - f"This pull-request is now outdated and has been closed in favour of {self.pr_url}" - ) - comment_request = requests.post( - url=list_prs_json[0]["comments_url"], - data=json.dumps({"body": comment_text}), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) + # Don't close the new PR that we just opened + if pr["head"]["ref"] == self.merge_branch: + continue - # Update the PR status to be closed - pr_update_api_url = list_prs_json[0]["url"] - pr_request = requests.patch( - url=pr_update_api_url, - data=json.dumps({"state": "closed"}), - auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), - ) - try: - pr_request_json = json.loads(pr_request.content) - pr_request_pp = json.dumps(pr_request_json, indent=4) - except: - pr_request_json = pr_request.content - pr_request_pp = pr_request.content - - # PR update worked - if pr_request.status_code == 200: - log.debug("GitHub API PR-update worked:\n{}".format(pr_request_pp)) - log.info("Closed GitHub PR: {}".format(pr_request_json["html_url"])) - return True - # Something went wrong - else: - log.warning(f"Could not close PR ('{pr_request.status_code}'):\n{pr_update_api_url}\n{pr_request_pp}") - return False + # 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): + """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." + ) + comment_request = requests.post( + url=pr["comments_url"], + data=json.dumps({"body": comment_text}), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), + ) + + # Update the PR status to be closed + pr_request = requests.patch( + url=pr["url"], + data=json.dumps({"state": "closed"}), + auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"]), + ) + try: + pr_request_json = json.loads(pr_request.content) + pr_request_pp = json.dumps(pr_request_json, indent=4) + except: + pr_request_json = pr_request.content + pr_request_pp = pr_request.content + # PR update worked + if pr_request.status_code == 200: + log.debug("GitHub API PR-update worked:\n{}".format(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 list open PRs ('{list_prs_request.status_code}')\n{list_prs_url}\n{list_prs_pp}") + log.warning(f"Could not close PR ('{pr_request.status_code}'):\n{pr['url']}\n{pr_request_pp}") + return False def reset_target_dir(self): """