Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues aren't filed for submodule sync errors #46

Open
CodingCanuck opened this issue May 10, 2022 · 3 comments
Open

Issues aren't filed for submodule sync errors #46

CodingCanuck opened this issue May 10, 2022 · 3 comments
Labels

Comments

@CodingCanuck
Copy link
Contributor

This automated PR to perform submodule sync failed: 3rdparty/eventuals#395

This should result in an issue being filed, which didn't happen.

The workflow log is here: https://github.com/3rdparty/eventuals/runs/6373939070?check_suite_focus=true

Two observations:

  1. the workflow claimed to succeed, even though log details show what I'd call a failure: the workflow should probably have failed to make these failed logs easier to spot
  2. the 'wait for build to succeed` section took 1h 0m 1s and is full of spam like this:
No completed checks named Build and Test, waiting for 10 seconds...
Retrieving check runs named Build and Test on 3rdparty/eventuals@917c6ee97c4ec399bb9115188b9a262aa1e70bf0...
Retrieved 0 check runs named Build and Test
[...]
No completed checks named Build and Test, waiting for 10 seconds...
No completed checks after 3600 seconds, exiting with conclusion 'timed_out'

That timeout should be an error.

Anyways, I suspect the timeout happened because the submodule sync tool expects a single workflow named "build and test" to complete: https://github.com/3rdparty/dev-tools/blob/main/.github/workflows/submodules-sync.yml#L177

The eventuals repo doesn't have a check named "build and test". It instead has several checks which have that string as a prefix:
Screen Shot 2022-05-10 at 7 59 33 PM

So I'd say we should:

  1. Make timeouts cause workflow failures
  2. Fix submodule sync to not expect a single hardcoded check name
  3. Somehow track / surface workflow failures (not sure if there's a good way to do this or if it's a turtles-all-the-way down problem with a workflow watching a workflow watching a workflow)

@rjhuijsman FYI

@CodingCanuck
Copy link
Contributor Author

This also needlessly delays submodule updates getting merged.

This PR has passed checks:
Screen Shot 2022-06-27 at 3 29 10 PM

Yet the submodule sync action is still running: https://github.com/reboot-dev/respect/runs/7084176082?check_suite_focus=true

because it's looking for a non-existent check:

No completed checks named Build and Test, waiting for 10 seconds...
Retrieving check runs named Build and Test on reboot-dev/respect@9d3ac3ac2a07ccc98126a0e22645ae46c3e697eb...
Retrieved 0 check runs named Build and Test
No completed checks named Build and Test, waiting for 10 seconds...

This will continue until the "wait for build to succeed" step times out.

@CodingCanuck
Copy link
Contributor Author

This also needlessly delays submodule updates getting merged.

This PR has passed checks: Screen Shot 2022-06-27 at 3 29 10 PM

Yet the submodule sync action is still running: reboot-dev/respect/runs/7084176082?check_suite_focus=true

because it's looking for a non-existent check:

No completed checks named Build and Test, waiting for 10 seconds...
Retrieving check runs named Build and Test on reboot-dev/respect@9d3ac3ac2a07ccc98126a0e22645ae46c3e697eb...
Retrieved 0 check runs named Build and Test
No completed checks named Build and Test, waiting for 10 seconds...

This will continue until the "wait for build to succeed" step times out.

A question: why do we wait for the build to succeed before adding mergequeue-ready? (Doesn't mergequeue already wait for checks to pass?)

@rjhuijsman
Copy link
Contributor

A question: why do we wait for the build to succeed before adding mergequeue-ready? (Doesn't mergequeue already wait for checks to pass?)

Not for any good reason that I'm aware of. My best guess is that we thought along the lines of "we have to wait for builds to pass/fail to decide what to do with issues, so we may as well only assign the label after it has passed".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants