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

Only return a failure exit code when all repos have failed #3369

Conversation

ioannakok
Copy link
Contributor

When even one repository fails in a scala steward run the workspace is not persisted by GitHub Actions because GH Actions won't persist the workspace of a failing job. Persisting workspace is crucial for respecting pullRequests.frequency configuration because the workspace is how scala steward records that it has opened a pull request. This means that one failing repository can cause a lot of user annoyance because their configuration is no longer respected and they get too many PRs opened. This change aims to fix that by only returning a failure code if all repos have failed. So long as at least one repository succeeded the exit code will be success and the workspace will persist. If administrators need to know what repos are failing they can use the jobs summary introduced by: #3071

See more here: guardian/scala-steward-public-repos#60

cc @alejandrohdezma

When even one repository fails in a scala steward run the workspace is not persisted by GitHub Actions because GH Actions won't persist the workspace of a failing job. Persisting workspace is crucial for respecting `pullRequests.frequency` configuration because the workspace is how scala steward records that it has opened a pull request. This means that one failing repository can cause a lot of user annoyance because their configuration is no longer respected and they get too many PRs opened.
This change aims to fix that by only returning a failure code if *all* repos have failed.
So long as at least one repository succeeded the exit code will be success and the workspace will persist. If administrators need to know what repos are failing they can use the jobs summary introduced by: scala-steward-org#3071

See more here: guardian/scala-steward-public-repos#60

Co-authored-by: Roberto Tyley <[email protected]>
@rtyley
Copy link
Contributor

rtyley commented Jun 5, 2024

@mzuehlke how does this look to you? To us at the Guardian this does seems like the best fix to the problem?

One possible alternative would be to modify the behaviour of scala-steward-action so that workspace.saveWorkspaceCache() gets invoked even if coursier.launch() throws an exception, though that probably runs the risk of persisting some inconsistent state.

@alejandrohdezma
Copy link
Member

Hey @ioannakok, thanks for this! I think this could make sense for a lot of use cases, but not for others. Maybe we can expand on it and put it behind a flag that's been sent in the params. That way we can propagate that flag from the action and people can decide whether to fail if everything is failing or not. What do you think?

rtyley added a commit to guardian/scala-steward that referenced this pull request Jun 11, 2024
The new `--exit-code-success-if-any-repo-succeeds` CLI flag alters the default
behaviour of the exit-code of the Scala Steward process - the exit code will be
success (`0`) if the processing of _any_ repo has succeeded, rather than
requiring that _all_ repos are successfully processed. This is useful when running
Scala Steward with the Scala Steward GitHub Action - it means that the filesystem
_will_ be persisted, along with the record of what PRs have been raised, which is
what you _want_ to happen, even if some repos have failed.

As Alejandro mentioned:

scala-steward-org#3369 (comment)

...this behaviour may not always be desired, so we're disabling it by default.
rtyley added a commit to guardian/scala-steward that referenced this pull request Jun 11, 2024
The new `--exit-code-success-if-any-repo-succeeds` CLI flag alters the default
behaviour of the exit-code of the Scala Steward process - the exit code will be
success (`0`) if the processing of _any_ repo has succeeded, rather than
requiring that _all_ repos are successfully processed. This is useful when running
Scala Steward with the Scala Steward GitHub Action - it means that the filesystem
_will_ be persisted, along with the record of what PRs have been raised, which is
what you _want_ to happen, even if some repos have failed.

As Alejandro mentioned:

scala-steward-org#3369 (comment)

...this behaviour may not always be desired, so we're disabling it by default.
@rtyley rtyley force-pushed the only-use-failure-exit-code-when-all-repos-have-failed branch from 49a28b2 to e8f6b9b Compare June 11, 2024 16:08
rtyley added a commit to guardian/scala-steward that referenced this pull request Jun 11, 2024
The new `--exit-code-success-if-any-repo-succeeds` CLI flag alters the default
behaviour of the exit-code of the Scala Steward process - the exit code will be
success (`0`) if the processing of _any_ repo has succeeded, rather than
requiring that _all_ repos are successfully processed. This is useful when running
Scala Steward with the Scala Steward GitHub Action - it means that the filesystem
_will_ be persisted, along with the record of what PRs have been raised, which is
what you _want_ to happen, even if some repos have failed.

As Alejandro mentioned:

scala-steward-org#3369 (comment)

...this behaviour may not always be desired, so we're disabling it by default.
@rtyley rtyley force-pushed the only-use-failure-exit-code-when-all-repos-have-failed branch from e8f6b9b to 8286c0f Compare June 11, 2024 16:11
The new `--exit-code-success-if-any-repo-succeeds` CLI flag alters the default
behaviour of the exit-code of the Scala Steward process - the exit code will be
success (`0`) if the processing of _any_ repo has succeeded, rather than
requiring that _all_ repos are successfully processed. This is useful when running
Scala Steward with the Scala Steward GitHub Action - it means that the filesystem
_will_ be persisted, along with the record of what PRs have been raised, which is
what you _want_ to happen, even if some repos have failed.

As Alejandro mentioned:

scala-steward-org#3369 (comment)

...this behaviour may not always be desired, so we're disabling it by default.
@rtyley rtyley force-pushed the only-use-failure-exit-code-when-all-repos-have-failed branch from 8286c0f to 0607cfc Compare June 11, 2024 16:46
@rtyley
Copy link
Contributor

rtyley commented Jun 11, 2024

I think this could make sense for a lot of use cases, but not for others. Maybe we can expand on it and put it behind a flag that's been sent in the params. That way we can propagate that flag from the action and people can decide whether to fail if everything is failing or not.

Thanks @alejandrohdezma - I've added a flag with 0607cfc, does that look ok?

@alejandrohdezma
Copy link
Member

Great work! Thanks! ❤️

@alejandrohdezma alejandrohdezma merged commit 9d9460a into scala-steward-org:main Jun 11, 2024
4 checks passed
@mzuehlke mzuehlke added the enhancement New feature or request label Jun 12, 2024
@mzuehlke mzuehlke added this to the 0.31.0 milestone Jun 12, 2024
Copy link
Member

@mzuehlke mzuehlke left a comment

Choose a reason for hiding this comment

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

Sorry for being absent, looks good 👍

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

Successfully merging this pull request may close these issues.

4 participants