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

Consider using GitHub merge queues #867

Open
2 tasks done
jezdez opened this issue Jan 8, 2024 · 3 comments
Open
2 tasks done

Consider using GitHub merge queues #867

jezdez opened this issue Jan 8, 2024 · 3 comments
Labels
backlog issue has been triaged but has not been earmarked for any upcoming release type::feature request for a new feature or capability

Comments

@jezdez
Copy link
Member

jezdez commented Jan 8, 2024

Checklist

  • I added a descriptive title
  • I searched open requests and couldn't find a duplicate

What is the idea?

We should consider using Github's merge queue for our pull requests to solve the problem of having to update all the time which leads to many additional CI runs.

Why is this needed?

To quote the docs:

The merge queue provides the same benefits as the Require branches to be up to date before merging branch protection, but does not require a pull request author to update their pull request branch and wait for status checks to finish before trying to merge.

That's possible because upon adding a pull request to the merge queue, another CI run is triggered that, if successful, will automatically merge into the main branch. And if unsuccessful shows up as a regular CI failure in the pull request.

The main advantage is that we would automate the coordination of merging potentially competing branches into main. The merge queue sequentially goes through the queue and builds all queued merges, but there is a way to bump a specific merge via an own user interface in case of an important change.

What should happen?

We should start by enabling the merge queue in our branch protection setup for the main branch (not release branches given the much less likely controversial changes).

We could also run longer running tests (optionally) for each merge queue item with different strategies, e.g.

  • only unit tests in pull request commits, the full integration tests in the merge queue
  • only linux in pull request commits, all other platforms in the merge queue
  • only the latest version of Python in the pull request commit, all other versions of Python in the merge queue

Additional Context

@jezdez jezdez added the type::feature request for a new feature or capability label Jan 8, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in 🧭 Planning Jan 8, 2024
@xhochy
Copy link

xhochy commented Jan 15, 2024

We could also run longer running tests (optionally) for each merge queue item with different strategies, e.g.

Do you have a working example of this? I'm a happy user of merge queue, but I would to understand on how to implement this correctly.

@jezdez
Copy link
Member Author

jezdez commented Jan 15, 2024

We could also run longer running tests (optionally) for each merge queue item with different strategies, e.g.

Do you have a working example of this? I'm a happy user of merge queue, but I would to understand on how to implement this correctly.

No working example in conda/*, I've only experimented based on what I've read about the merge_group event. Generally speaking it, I imagine a separate workflow file that would only listen to the merge_group event and not the pull_request or push events, which would run additional tests, e.g. integration tests.

@dbast
Copy link
Member

dbast commented Jan 17, 2024

There is a lot to say to this:

  • long running test suites:
    • the current conda tests.yaml spawns 74 jobs taking ruffly all in all between 40 minutes and 2.5 hours, depending on the current resource consumptions in the GH org
    • the resource usage could be improved chaining the test jobs: meaning only if maybe pre-commit (to be run as gh wokflow then) and some specific smoke-tests / unit-test suites are green then all the remaining jobs are triggered... so broken PRs don't consume that many resource if pre-commit and a small set of short running smoke/unit tests are able to find the majority of the bugs
    • Good thing of the many job is: they can be individually retriggered if flaky.
  • updating branches before merging:
    • 1 Github by default virtually merges branches of PR to the target branch, that produces the same situation as merging in the target branch into the PR branch.
    • 2 Github also detects conflicts that prevent a merge.
    • 1+2 give some high probability that a merge will run fine, without forcing to merge the target branch....
    • If PR "a" got merged and tests of PR "b" did run before the merge then one could check if there are conflicts (merge not possible) or rerun some of the tests via retrigger or merge the target branch / close-reopen the PR (virtual merge then archives the same)... this is then a human decision
    • Running pre-commit via the merge queue would at least cover some obvious finding via linting
    • This is anyway not that critical as merging to main IS NOT release to production... a release is a separate human action...
  • running long running tests via merge queue or the human in the loop and approval fatigue:
    • this leads to approving PRs just to see what the long tests / complete test suite say: a PR get approved, but fails during merge queue, can be either a flaky test job (which can afaik not be re-run as part of the merge queue!) or a minor PR update is required... requires another approval + merge + waiting 40 mins to 2.5 hours ...
    • this can cause approval fatigue as PRs are tried to be merged to get the desired feedback from tests
    • this also inverts human decision making: the reviewer(s) normally check all the test/lint runs, looks at the code and make an ultimate decision to merge... pressing the button leads to an imidiate merge, without waiting time
    • with the testsuite run in the merge queue, the ultimate decisions are the tests, not the human decision that happened before.... so flaky tests (and lack of possibility to re-run those) become a big issue.
    • a super reliable and fast workflow (pre-commit) could help to keep the human in the ultimate decision position, as fixing findings are in that case are mostly fine and folks don't feel powerless waiting for an 2h test suite that maybe could fail.

tldr; running pre-commit via merge-queue is nice (and also fast), requiring branch to be up to date can be removed then, and job chaining can save resources ... I personally enjoy merge queues with 1-2 minutes lengths, even 20sec more or less have consequences to how productive one feels.

@kenodegard kenodegard added the backlog issue has been triaged but has not been earmarked for any upcoming release label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog issue has been triaged but has not been earmarked for any upcoming release type::feature request for a new feature or capability
Projects
Status: 🆕 New
Development

No branches or pull requests

4 participants