Work around merge failures in subsequent CI attempts #882
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The migration to the v4 version of the artifact API has somewhat broken rerunning tests for a PR in some scenarios because now artifacts are scoped to the attempt, but it's possible that we won't generate every type of artifact on every attempt (e.g. if all of our notebook tests passes in attempt 1, we won't try to run them again in attempt 2, see, e.g. https://github.com/py-why/EconML/actions/runs/8940985090/attempts/2 for a concrete example).
Unfortunately, the new merge artifact action's API doesn't provide a graceful way to handle this at the moment, so we can just set the step's
continue-on-error
property to true to ignore failures (ideally we wouldn't have to assume that this is the only reason the job can fail, but I don't see any obvious alternative).Note that this isn't exactly a blocker, because our
Verify CI checks
job doesn't look for failures in the merge jobs anyway, but it's potentially confusing (IMO) that we can have red Xs on some jobs for the attempt and yet still merge the PR and so it would be better to have the merge jobs not fail in this case. (There is some other semantic weirdness with the fact that artifacts are per-attempt, like the fact that we want the final coverage report to ideally include just the last (successful) run for each test configuration, but instead we'll get one separate coverage report per attempt that includes only the tests that were run in that attempt; again, there does not seem to be a clean workaround at the moment).