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

Add concurrency key to publish/preview workflows #527

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Oct 1, 2024

This PR adds the concurrency key docs to both publish and preview workflows, preventing them from running concurrently. This will help us avoid weird race conditions like the one described in #526, which came about because the preview workflow overwrote changes from the publish workflow.

Additionally, the publish workflow is also given cancel-in-progress: true meaning that if a preview workflow is running, starting a publish workflow will cancel the preview workflow.

This is probably OK because preview workflows run on PRs, and if somebody pushes to main (which triggers the publish workflow), the PR has to incorporate the new changes anyway.

The only downside to this is that there can only be one queued job at any time. So if (for example) three people are working on PRs at the same time and trigger preview workflows, one workflow would run, one workflow would be queued, and one of them would be cancelled. That scenario seems quite unlikely given the level of contributions to the docs, though, and can be fixed fairly easily by manually rerunning the workflow. (This is a weird limitation of GHA, and apparently a fix for this is on GitHub's roadmap, see https://github.com/orgs/community/discussions/41518.)

Closes #526.

CI is failing because I manually cancelled the preview workflow on this PR (it doesn't serve any purpose).

@shravanngoswamii
Copy link
Member

That scenario seems quite unlikely given the level of contributions to the docs, though, and can be fixed fairly easily by manually rerunning the workflow.

Quite agreeable!

Copy link
Member

@shravanngoswamii shravanngoswamii left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you, @penelopeysm!

@penelopeysm penelopeysm merged commit 5255f44 into master Oct 2, 2024
2 of 4 checks passed
@penelopeysm penelopeysm deleted the py/concurrency branch October 2, 2024 16:13
penelopeysm added a commit that referenced this pull request Nov 26, 2024
penelopeysm added a commit that referenced this pull request Nov 26, 2024
@penelopeysm penelopeysm mentioned this pull request Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changes from #522 don't appear on deployed docs(?)
2 participants