From 7d32364c54373daecbbc98c9003e0c5e9c48e6bd Mon Sep 17 00:00:00 2001 From: Javier Gonzalez Date: Thu, 26 Oct 2023 14:13:23 -0400 Subject: [PATCH 1/5] do not crash when packages have no PRs or no commits --- skare3_tools/packages.py | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/skare3_tools/packages.py b/skare3_tools/packages.py index 7ba661e..5f4be77 100755 --- a/skare3_tools/packages.py +++ b/skare3_tools/packages.py @@ -518,25 +518,29 @@ def _get_repository_info_v4( commits_path = "data/repository/defaultBranchRef/target/history" commits = data_v4[commits_path]["nodes"] - commits += get_all_nodes( - owner, - name, - commits_path, - _COMMIT_QUERY, - reverse=False, - at=data_v4[commits_path]["pageInfo"]["endCursor"], - ) + if data_v4[commits_path]["pageInfo"]["endCursor"] is not None: + # append the rest of the commits only if there were commits to begin with + commits += get_all_nodes( + owner, + name, + commits_path, + _COMMIT_QUERY, + reverse=False, + at=data_v4[commits_path]["pageInfo"]["endCursor"], + ) pull_requests_path = "data/repository/pullRequests" pull_requests = data_v4[pull_requests_path]["nodes"] - pull_requests += get_all_nodes( - owner, - name, - pull_requests_path, - _PR_QUERY, - reverse=True, - at=data_v4[pull_requests_path]["pageInfo"]["startCursor"], - ) + if data_v4[pull_requests_path]["pageInfo"]["startCursor"] is not None: + # append the rest of the PRs only if there were commits to begin with + pull_requests += get_all_nodes( + owner, + name, + pull_requests_path, + _PR_QUERY, + reverse=True, + at=data_v4[pull_requests_path]["pageInfo"]["startCursor"], + ) # from now, keep a list of the open pull requests on the main branch all_pull_requests = {pr["number"]: pr for pr in pull_requests} From b7391118075e992345be046eb650929541353640 Mon Sep 17 00:00:00 2001 From: Javier Gonzalez Date: Thu, 26 Oct 2023 14:13:46 -0400 Subject: [PATCH 2/5] do not crash if a package has no releases --- skare3_tools/packages.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/skare3_tools/packages.py b/skare3_tools/packages.py index 5f4be77..6e1a6f0 100755 --- a/skare3_tools/packages.py +++ b/skare3_tools/packages.py @@ -593,16 +593,26 @@ def _get_repository_info_v4( + "({release_tags})".format(release_tags=release_tags) ) - rel_commits = get_all_nodes( - owner, - name, - "data/repository/ref/compare/commits", - _COMPARE_COMMITS_QUERY, - reverse=False, - base=releases[0]["tagName"], - head=default_branch, - ) - rel_prs = _pr_commits(rel_commits, all_pull_requests, use_pr_titles=use_pr_titles) + if len(releases) == 0: + # if there are no releases, look for merge messages in all commits + rel_prs = _pr_commits(commits, all_pull_requests, use_pr_titles=use_pr_titles) + else: + # if there are releases, look for merge messages in the commits since the last release + rel_commits = get_all_nodes( + owner, + name, + "data/repository/ref/compare/commits", + _COMPARE_COMMITS_QUERY, + reverse=False, + base=releases[0]["tagName"], + head=default_branch, + ) + rel_prs = _pr_commits( + rel_commits, all_pull_requests, use_pr_titles=use_pr_titles + ) + + # the first entry in release_info does not correspond to a release + # it's the list of PRs (and commits) waiting to be released. release_info = [ { "release_tag": "", From 7e3d3ee0275d4362a19430396ecf1701f2418009 Mon Sep 17 00:00:00 2001 From: Javier Gonzalez Date: Tue, 21 Nov 2023 10:18:43 -0500 Subject: [PATCH 3/5] fix template causing infinite loop --- skare3_tools/packages.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/skare3_tools/packages.py b/skare3_tools/packages.py index 6e1a6f0..2409e8f 100755 --- a/skare3_tools/packages.py +++ b/skare3_tools/packages.py @@ -347,7 +347,7 @@ def _get_tag_target(tag): compare(headRef: "{{ head }}") { aheadBy behindBy - commits(first: 100, after: "{{ after }}") { + commits(first: 100, after: "{{ cursor }}") { nodes { oid message @@ -443,6 +443,9 @@ def get_all_nodes( if query_2 is None: query_2 = query while data[path]["pageInfo"][has_more]: + if at == data[path]["pageInfo"][cursor]: + raise RuntimeError('Cursor did not change and will cause an infinite loop') + at = data[path]["pageInfo"][cursor] data = Dict( github.GITHUB_API_V4( From 6190554413ab12074f0673406762d21afd5685cf Mon Sep 17 00:00:00 2001 From: Javier Gonzalez Date: Fri, 5 Jan 2024 12:22:14 -0500 Subject: [PATCH 4/5] improve matching between commits and PRs so it does not depend on commit message, and removed use_pr_titles argument all over (titles are always used now) --- skare3_tools/github/graphql.py | 3 ++ skare3_tools/packages.py | 61 ++++++++++++++++------------------ 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/skare3_tools/github/graphql.py b/skare3_tools/github/graphql.py index 2f4a3fd..ad1eed5 100644 --- a/skare3_tools/github/graphql.py +++ b/skare3_tools/github/graphql.py @@ -263,6 +263,9 @@ class AuthException(Exception): number title url + mergeCommit { + oid + } commits(last: 100) { totalCount nodes { diff --git a/skare3_tools/packages.py b/skare3_tools/packages.py index 2409e8f..55128a8 100755 --- a/skare3_tools/packages.py +++ b/skare3_tools/packages.py @@ -309,6 +309,9 @@ def _get_tag_target(tag): number title url + mergeCommit { + oid + } commits(last: 100) { totalCount nodes { @@ -468,33 +471,40 @@ def check_api_errors(data): raise Exception(msg) -def _pr_commits(commits, all_pull_requests, use_pr_titles=True): +def _pr_commits(commits, all_pull_requests): merges = [] + pulls_v_hash = { + pr["mergeCommit"]["oid"]: pr + for pr in all_pull_requests.values() + if pr["mergeCommit"] is not None + } for commit in commits: match = re.match( r"Merge pull request #(?P.+) from (?P\S+)(\n\n(?P.+))?", commit["message"], ) - if match: + if commit["oid"] in pulls_v_hash: + merge = { + "pr_number": pulls_v_hash[commit["oid"]]["number"], + "title": pulls_v_hash[commit["oid"]]["title"], + "branch": pulls_v_hash[commit["oid"]]["headRefName"], + "author": pulls_v_hash[commit["oid"]]["author"]["name"], + } + merges.append(merge) + elif match: + # I don't think it will ever enter this branch + # this would be recognizable in the dashboard because the PR author is unknown merge = match.groupdict() merge["pr_number"] = int(merge["pr_number"]) + merge["author"] = "Unknown" merges.append(merge) - for merge in merges: - merge["author"] = "Unknown" - if merge["pr_number"] in all_pull_requests: - merge["author"] = all_pull_requests[merge["pr_number"]]["author"]["name"] - if use_pr_titles or merge["title"] is None: - # some times PR titles are changed after merging. Use that instead of the commit - merge["title"] = all_pull_requests[merge["pr_number"]]["title"] - return merges def _get_repository_info_v4( owner_repo, since=7, - use_pr_titles=True, include_unreleased_commits=False, include_commits=False, ): @@ -598,7 +608,7 @@ def _get_repository_info_v4( if len(releases) == 0: # if there are no releases, look for merge messages in all commits - rel_prs = _pr_commits(commits, all_pull_requests, use_pr_titles=use_pr_titles) + rel_prs = _pr_commits(commits, all_pull_requests) else: # if there are releases, look for merge messages in the commits since the last release rel_commits = get_all_nodes( @@ -610,9 +620,7 @@ def _get_repository_info_v4( base=releases[0]["tagName"], head=default_branch, ) - rel_prs = _pr_commits( - rel_commits, all_pull_requests, use_pr_titles=use_pr_titles - ) + rel_prs = _pr_commits(rel_commits, all_pull_requests) # the first entry in release_info does not correspond to a release # it's the list of PRs (and commits) waiting to be released. @@ -636,9 +644,7 @@ def _get_repository_info_v4( base=base["tagName"], head=head["tagName"], ) - rel_prs = _pr_commits( - rel_commits, all_pull_requests, use_pr_titles=use_pr_titles - ) + rel_prs = _pr_commits(rel_commits, all_pull_requests) release = { "release_sha": head["tag_oid"], "release_commit_date": head["committed_date"], @@ -830,7 +836,6 @@ def _get_release_commit(repository, release_name): def _get_repository_info_v3( owner_repo, since=7, - use_pr_titles=True, include_unreleased_commits=False, include_commits=False, ): @@ -844,9 +849,6 @@ def _get_repository_info_v3( :param since: int or str the maximum number of releases to look back, or the release tag to look back to (not inclusive). - :param use_pr_titles: bool - Whether to use PR titles instead of the PR commit message. - This is so one can change PR titles after the fact to be more informative. :param include_unreleased_commits: bool whether to include commits and merges for repositories that have no release. This affects only top-level entries 'commits', 'merges', 'merge_info'. @@ -893,9 +895,8 @@ def _get_repository_info_v3( {"release_tag": "", "release_tag_date": "", "commits": [], "merges": []} ] - if use_pr_titles: - all_pull_requests = repository.pull_requests(state="all") - all_pull_requests = {pr["number"]: pr for pr in all_pull_requests} + all_pull_requests = repository.pull_requests(state="all") + all_pull_requests = {pr["number"]: pr for pr in all_pull_requests} commits = repository.commits( sha=repository.info["default_branch"], since=date_since ) @@ -931,11 +932,8 @@ def _get_repository_info_v3( if match: merge = match.groupdict() merge["pr_number"] = int(merge["pr_number"]) - if use_pr_titles: - if merge["pr_number"] in all_pull_requests: - merge["title"] = all_pull_requests[merge["pr_number"]][ - "title" - ].strip() + if merge["pr_number"] in all_pull_requests: + merge["title"] = all_pull_requests[merge["pr_number"]]["title"].strip() release_info[-1]["merges"].append(merge) if len(release_info) > 1: @@ -1041,9 +1039,6 @@ def get_repository_info(owner_repo, version="v4", **kwargs): :param since: int or str the maximum number of releases to look back, or the release tag to look back to (not inclusive). - :param use_pr_titles: bool - Whether to use PR titles instead of the PR commit message. - This is so one can change PR titles after the fact to be more informative. :param include_unreleased_commits: bool whether to include commits and merges for repositories that have no release. This affects only top-level entries 'commits', 'merges', 'merge_info'. From d6fe3947af186e9cc4e7141b6f14105ead978747 Mon Sep 17 00:00:00 2001 From: Javier Gonzalez <javierggt@yahoo.com> Date: Fri, 5 Jan 2024 12:48:57 -0500 Subject: [PATCH 5/5] fixup --- skare3_tools/packages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skare3_tools/packages.py b/skare3_tools/packages.py index 55128a8..dc2bb4d 100755 --- a/skare3_tools/packages.py +++ b/skare3_tools/packages.py @@ -447,7 +447,7 @@ def get_all_nodes( query_2 = query while data[path]["pageInfo"][has_more]: if at == data[path]["pageInfo"][cursor]: - raise RuntimeError('Cursor did not change and will cause an infinite loop') + raise RuntimeError("Cursor did not change and will cause an infinite loop") at = data[path]["pageInfo"][cursor] data = Dict(