git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] ci: make the new GitHub workflow logic more defensive
@ 2020-10-11 21:27 Johannes Schindelin via GitGitGadget
  2020-10-11 21:27 ` [PATCH 1/2] ci: work around old records of GitHub runs Johannes Schindelin via GitGitGadget
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-10-11 21:27 UTC (permalink / raw)
  To: git; +Cc: Philippe Blain, Johannes Schindelin

Phil just reported that the check I added failed miserably
[https://github.com/phil-blain/git/runs/1238660958] in their fork, and the 
entire run would be abandoned. That's totally unintended, and was caused by
a different metadata format for older recorded runs.

Let's work around this particular issue, and then make the entire check a
lot more robust.

This patch series is based on js/ci-ghwf-dedup-tests.

Johannes Schindelin (2):
  ci: work around old records of GitHub runs
  ci: make the "skip-if-redundant" check more defensive

 .github/workflows/main.yml | 58 ++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 27 deletions(-)


base-commit: 4463ce75b7eea47f9b484b05957def655d3f46d5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-748%2Fdscho%2Fci-ghwf-dedup-tests-defensive-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-748/dscho/ci-ghwf-dedup-tests-defensive-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/748
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] ci: work around old records of GitHub runs
  2020-10-11 21:27 [PATCH 0/2] ci: make the new GitHub workflow logic more defensive Johannes Schindelin via GitGitGadget
@ 2020-10-11 21:27 ` Johannes Schindelin via GitGitGadget
  2020-10-11 21:27 ` [PATCH 2/2] ci: make the "skip-if-redundant" check more defensive Johannes Schindelin via GitGitGadget
  2020-10-12 19:27 ` [PATCH 0/2] ci: make the new GitHub workflow logic " Junio C Hamano
  2 siblings, 0 replies; 5+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-10-11 21:27 UTC (permalink / raw)
  To: git; +Cc: Philippe Blain, Johannes Schindelin, Johannes Schindelin

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Apparently older GitHub runs at least _sometimes_ lack information about
the `head_commit` (and therefore the `ci-config` check will fail with
"TypeError: Cannot read property 'tree_id' of null") in the check added
in 7d78d5fc1a9 (ci: skip GitHub workflow runs for already-tested
commits/trees, 2020-10-08).

Let's work around this by adding a defensive condition.

Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/main.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0a9acb6a19..d4298878f5 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -65,7 +65,7 @@ jobs:
                 core.setOutput('enabled', ' but skip');
                 break;
               }
-              if (tree_id === run.head_commit.tree_id) {
+              if (run.head_commit && tree_id === run.head_commit.tree_id) {
                 core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`);
                 core.setOutput('enabled', ' but skip');
                 break;
-- 
gitgitgadget


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] ci: make the "skip-if-redundant" check more defensive
  2020-10-11 21:27 [PATCH 0/2] ci: make the new GitHub workflow logic more defensive Johannes Schindelin via GitGitGadget
  2020-10-11 21:27 ` [PATCH 1/2] ci: work around old records of GitHub runs Johannes Schindelin via GitGitGadget
@ 2020-10-11 21:27 ` Johannes Schindelin via GitGitGadget
  2020-10-12 19:29   ` Junio C Hamano
  2020-10-12 19:27 ` [PATCH 0/2] ci: make the new GitHub workflow logic " Junio C Hamano
  2 siblings, 1 reply; 5+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-10-11 21:27 UTC (permalink / raw)
  To: git; +Cc: Philippe Blain, Johannes Schindelin, Johannes Schindelin

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In 7d78d5fc1a9 (ci: skip GitHub workflow runs for already-tested
commits/trees, 2020-10-08), we added a check that determines whether
there is already a workflow run for the given commit (or at least tree),
and if found, skips the current run.

We just worked around an issue with this check where older runs might
unexpectedly miss the `head_commit` attribute.

Let's be even more defensive by catching all kinds of exceptions,
logging them as warnings, and continue the run without skipping it
(after all, if the check fails, we _want_ to continue with the run).

This commit is best viewed with the diff option `-w` because it
increases the indentation level of the GitHub Action script by two
spaces, surrounding it by a `try ... catch` construct.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/main.yml | 58 ++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index d4298878f5..9dc359bc23 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -41,35 +41,39 @@ jobs:
         with:
           github-token: ${{secrets.GITHUB_TOKEN}}
           script: |
-            // Figure out workflow ID, commit and tree
-            const { data: run } = await github.actions.getWorkflowRun({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              run_id: context.runId,
-            });
-            const workflow_id = run.workflow_id;
-            const head_sha = run.head_sha;
-            const tree_id = run.head_commit.tree_id;
+            try {
+              // Figure out workflow ID, commit and tree
+              const { data: run } = await github.actions.getWorkflowRun({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                run_id: context.runId,
+              });
+              const workflow_id = run.workflow_id;
+              const head_sha = run.head_sha;
+              const tree_id = run.head_commit.tree_id;
 
-            // See whether there is a successful run for that commit or tree
-            const { data: runs } = await github.actions.listWorkflowRuns({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              per_page: 500,
-              status: 'success',
-              workflow_id,
-            });
-            for (const run of runs.workflow_runs) {
-              if (head_sha === run.head_sha) {
-                core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`);
-                core.setOutput('enabled', ' but skip');
-                break;
-              }
-              if (run.head_commit && tree_id === run.head_commit.tree_id) {
-                core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`);
-                core.setOutput('enabled', ' but skip');
-                break;
+              // See whether there is a successful run for that commit or tree
+              const { data: runs } = await github.actions.listWorkflowRuns({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                per_page: 500,
+                status: 'success',
+                workflow_id,
+              });
+              for (const run of runs.workflow_runs) {
+                if (head_sha === run.head_sha) {
+                  core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`);
+                  core.setOutput('enabled', ' but skip');
+                  break;
+                }
+                if (run.head_commit && tree_id === run.head_commit.tree_id) {
+                  core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`);
+                  core.setOutput('enabled', ' but skip');
+                  break;
+                }
               }
+            } catch (e) {
+              core.warning(e);
             }
 
   windows-build:
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] ci: make the new GitHub workflow logic more defensive
  2020-10-11 21:27 [PATCH 0/2] ci: make the new GitHub workflow logic more defensive Johannes Schindelin via GitGitGadget
  2020-10-11 21:27 ` [PATCH 1/2] ci: work around old records of GitHub runs Johannes Schindelin via GitGitGadget
  2020-10-11 21:27 ` [PATCH 2/2] ci: make the "skip-if-redundant" check more defensive Johannes Schindelin via GitGitGadget
@ 2020-10-12 19:27 ` Junio C Hamano
  2 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2020-10-12 19:27 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: git, Philippe Blain, Johannes Schindelin

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Phil just reported that the check I added failed miserably
> [https://github.com/phil-blain/git/runs/1238660958] in their fork, and the 
> entire run would be abandoned. That's totally unintended, and was caused by
> a different metadata format for older recorded runs.

Thanks for a quick response to breakage.

> Let's work around this particular issue, and then make the entire check a
> lot more robust.
>
> This patch series is based on js/ci-ghwf-dedup-tests.

Will queue.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] ci: make the "skip-if-redundant" check more defensive
  2020-10-11 21:27 ` [PATCH 2/2] ci: make the "skip-if-redundant" check more defensive Johannes Schindelin via GitGitGadget
@ 2020-10-12 19:29   ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2020-10-12 19:29 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: git, Philippe Blain, Johannes Schindelin

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Let's be even more defensive by catching all kinds of exceptions,
> logging them as warnings, and continue the run without skipping it
> (after all, if the check fails, we _want_ to continue with the run).

Yeah, looks sensible.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-12 19:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-11 21:27 [PATCH 0/2] ci: make the new GitHub workflow logic more defensive Johannes Schindelin via GitGitGadget
2020-10-11 21:27 ` [PATCH 1/2] ci: work around old records of GitHub runs Johannes Schindelin via GitGitGadget
2020-10-11 21:27 ` [PATCH 2/2] ci: make the "skip-if-redundant" check more defensive Johannes Schindelin via GitGitGadget
2020-10-12 19:29   ` Junio C Hamano
2020-10-12 19:27 ` [PATCH 0/2] ci: make the new GitHub workflow logic " Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git