Avoid duplicating CI runs on pull requests. #3950
Conversation
The CI workflow for devdocs really hammers the GitHub API because of the way devdocs works currently — sometimes so much that it fails because of rate-limiting — so it's worth avoiding running duplicate jobs. This is the simplest way to avoid duplicate workflow runs; the only downside is that we'll no longer automatically trigger on third-party pull requests — but our [process for handling external contributions](https://docs.publishing.service.gov.uk/manual/merge-pr.html#a-change-from-an-external-contributor) already covers that. If we want to make it automatic for external contributions in future, that's entirely doable — just involves some slightly more fancy YAML.
|
Quick drive-by comment - we did once cut down to one run instead of two, but that got reverted in #3818 for reasons outlined in the PR. In that instance, it was |
|
Yeah ends up similar situation as previous PR. If we drop This also goes against what we have documented as our practice for GitHub Actions in: https://docs.publishing.service.gov.uk/manual/test-and-build-a-project-with-github-actions.html#when-the-ci-workflow-should-run as per RFC 123 It is becoming increasing clear we need a new agreed approach as all the CI files replatforming created don't use the documented approach, so we have a mixture across our stack. It's clear that people don't find The compromise I'd suggest we take is we have:
I imagine that'd cover all the use cases and remove the duplicate run concern |
|
@kevindew - was just a bit confused about when you said:
Do you mean if we've only got |
Nope, that does get triggered then What I meant that for a given commit |
|
Right now the situation is pretty bad — I was unable to make a simple documentation change last week because the tests started failing because all the duplicate runs caused us to hit a GitHub ratelimit. I understand there's plenty more room for improvement here, but it's important that we don't let the lack of an ideal solution prevent us from fixing real, practical problems that we have right now.
Aren't those always going to be the same, provided the branch is reasonably up-to-date? (and there's an option to require that, if need be) |
|
I definitely want to understand if this change would cause some important regression that I've missed. If not though, I think we need to not let perfect be the enemy of good — otherwise people are gonna be discouraged from improving documentation. |
|
+1 to adding a manual trigger ( |
They're only the same if the branch is ahead of main, otherwise it's the result of the merge.
The main problem I see with this is it creates a fresh inconsistency, so we will now have a third approach to GitHub Action CI triggering and it again deviates from the documented approach. I can understand your frustration, but I think just moving ahead with this makes us further away from consistency. If you do just want to get something done quick it'd at least make more sense to me to use the convention used in app CI files rather than create a new one. I think we just need to agree a team that is responsible for proposing an approach (Platform Reliability or Replatforming), getting some feedback and documenting the agreement |
|
I think we need to be careful about our overall goal here. What problem are we trying to solve via "consistency"? |
A quick list I'd have is: surprise when working in different projects, superfluous discussions about approaches (like this) because there's no consistency, ability to quickly tell when project is configured "right" or "wrong" Can you think why we wouldn't want to be consistent in our approach to when CI runs? |
This has been produced following the discussiona in: #3950 and #3816 This is to try resolve a consistency issue we have in GOV.UK where we have an agreed approach to using GitHub Actions documented which doesn't reflect the configurations used across GOV.UK. Since we agreed the approach of running CI GitHub Action on `[push, pull_request]` and adopted it has become clear that this is not unanimously popular and, due to the appearance of duplicate jobs, can imply misconfiguration. The motivation to document a new approach is to try form consensus on an approach that can be good enough for most cases and cover common concerns. This can then reduce the time we spend debating how to configure individual projects. Aspects I've wanted to cover in this are: - Stay broadly consistent with approach replatforming popularised since that is what is predominantly in use - Remove the appearance of duplicate runs by default in pull requests - Allow CI for external contributors (our [current process for dealing with them][1] seems more of a legacy Jenkins concern) - Offer an approach for running CI without opening a PR, as that seems to be a common point of contention (it's a key part of some workflows, yet not a concern for others). A risk of this approach is that the use of workflow_dispatch is somewhat ignored and thus unreliable. This doesn't seem a huge concern to me as I imagine this would be fixed if it's used frequently or be a sign that it can be removed. [1]: https://docs.publishing.service.gov.uk/manual/merge-pr.html#a-change-from-an-external-contributor
|
@sengi @ChrisBAshton @theseanything I've had a go at implementing and documenting a consistent approach. Could you take a look and let me know if you have concerns with it? If not, I'll share more broadly. |
|
What do I need to do to make this change acceptable? If it's a question of keeping all the repos the same then I don't mind applying the same change everywhere else. If there's a problem with the logic of the change (leaving aside the question of the other repos for a moment) then what would be an acceptable fix? TIA! 🙇 |
|
Oh, thanks! We commented at almost exactly the same time. Snap! 😅 I'll take a look now :) |
This has been produced following the discussiona in: #3950 and #3816 This is to try resolve a consistency issue we have in GOV.UK where we have an agreed approach to using GitHub Actions documented which doesn't reflect the configurations used across GOV.UK. Since we agreed the approach of running CI GitHub Action on `[push, pull_request]` and adopted it has become clear that this is not unanimously popular and, due to the appearance of duplicate jobs, can imply misconfiguration. The motivation to document a new approach is to try form consensus on an approach that can be good enough for most cases and cover common concerns. This can then reduce the time we spend debating how to configure individual projects. Aspects I've wanted to cover in this are: - Stay broadly consistent with approach replatforming popularised since that is what is predominantly in use - Remove the appearance of duplicate runs by default in pull requests - Allow CI for external contributors (our [current process for dealing with them][1] seems more of a legacy Jenkins concern) - Offer an approach for running CI without opening a PR, as that seems to be a common point of contention (it's a key part of some workflows, yet not a concern for others). A risk of this approach is that the use of workflow_dispatch is somewhat ignored and thus unreliable. This doesn't seem a huge concern to me as I imagine this would be fixed if it's used frequently or be a sign that it can be removed. [1]: https://docs.publishing.service.gov.uk/manual/merge-pr.html#a-change-from-an-external-contributor
This has been produced following the discussiona in: #3950 and #3816 This is to try resolve a consistency issue we have in GOV.UK where we have an agreed approach to using GitHub Actions documented which doesn't reflect the configurations used across GOV.UK. Since we agreed the approach of running CI GitHub Action on `[push, pull_request]` and adopted it has become clear that this is not unanimously popular and, due to the appearance of duplicate jobs, can imply misconfiguration. The motivation to document a new approach is to try form consensus on an approach that can be good enough for most cases and cover common concerns. This can then reduce the time we spend debating how to configure individual projects. Aspects I've wanted to cover in this are: - Stay broadly consistent with approach replatforming popularised since that is what is predominantly in use - Remove the appearance of duplicate runs by default in pull requests - Allow CI for external contributors (our [current process for dealing with them][1] seems more of a legacy Jenkins concern) - Offer an approach for running CI without opening a PR, as that seems to be a common point of contention (it's a key part of some workflows, yet not a concern for others). A risk of this approach is that the use of workflow_dispatch is somewhat ignored and thus unreliable. This doesn't seem a huge concern to me as I imagine this would be fixed if it's used frequently or be a sign that it can be removed. [1]: https://docs.publishing.service.gov.uk/manual/merge-pr.html#a-change-from-an-external-contributor
The CI workflow for devdocs really hammers the GitHub API because of the way devdocs works currently — sometimes so much that it fails because of rate-limiting — so it's worth avoiding running duplicate jobs.
This is the simplest way to avoid duplicate workflow runs; the only downside is that we'll no longer automatically trigger on third-party pull requests — but our process for handling external contributions already covers that.
If we want to make it automatic for external contributions in future, that's entirely doable — just involves some slightly more fancy YAML (hence not doing it in this PR, because I'm already two yaks deep and I just want to stop the tests from flaking).