By re-organizing the flow in `handle()` we can start labeling both
issues and pull requests, and only make the relevant API requests for
the PR-case.
At first glance, we might think that we only need to label the big batch
list of issues and not those recently updated: But that's wrong, for
recently updated issues it's important to label quickly, because the
stale label needs to be *removed*, too.
We already tried to fix this case earlier, but didn't account for all
cases: A scheduled workflow can also encounter a pull request with
failed PR workflow. This failure doesn't need to be in the Eval part, so
artifacts could *still* be available. To make sure PRs always get
rebuild labels, just ignore the status condition. Either the artifact is
there, or it is not.
The previous implementation had two problems:
- When switching from /search to /pulls, we disabled the additional GET
on each single pull request - which causes no test merge commit creation
for all PRs. This means, merge conflicts will not actually be detected.
- By using `item` in the pull-request triggered case, this goes back to
`context.payload.pull_request`, which is the state *at the beginning* of
the workflow run. But this renders our "let's wait 3 minutes before
checking merge_commit_sha" logic void. While we wait for 3 minutes, we
still use the *old* value afterwards...
Just making the extra request every time simplifies the logic and solves
both problems.
It's necessary to use a combination of different endpoints here, because
the /search endpoint only allows fetching the first 1000 items and will
fail with a higher page number (11+). On the flip side, the /pulls
endpoint doesn't allow counting the total number of results, so we can't
calculate the required page number with its response.
Putting both together should work, though.
This should make sure that the timer is cleaned up, no matter what. This
didn't seem to be the case before, where it would still be stuck
sometimes, when throwing an error somewhere.
When running in a pull_request context, the labels job is part of the
currently running workflow - which will never have succeeded, yet.
Apparently it could be failed already, so in this case we take *any*
workflow run, no matter its state.
This manages the `2. status: stale` label for pull requests only (not
issues, yet) with the following conditions:
- The last event on the timeline of the Pull Request counts.
- Labeling and unlabeling of any kind are ignored.
- Older than 180 days are stale.
- Security labeled PRs are never stale.
To handle this label correctly, it's important to go through all pull
requests. Any approach to limit the list of PRs via search are not going
to work:
- Filtering by `updated` is not going to work, because it includes the
last time that *a label was set* on the PR. To actually find out whether
a PR is stale or not, the timeline of events needs to be looked at.
- Filtering by an existing stale label is not going to work either,
because such a label might have been added manually and thus breaking
the rules we set up here. Thus any existing label needs to be confirmed
as well.
We keep working through the PR, even though we don't have any eval
results. This will allow actually managing labels for much older PRs as
well. Most importantly, it will allow merge-conflict and stale-labeling
next.
This replaces the manual dispatch trigger with a batched run through all
pull requests every day. This has the small benefit of not having to
worry about backfilling labeling after fixing bugs - and the much bigger
one in being able to handle merge-conflict and stale labels properly
later. For those, it's inevitable to eventually scan through all PRs.
At this stage, the vast majority of PRs will still be skipped, because
there won't be an eval run with artifact available. This will be
improved in the next step.
Technically, the workflow_dispatch trigger is kept to allow easily
testing this in forks, where the scheduled jobs are disabled. The
triggered job will behave similar to the scheduled job, though, and have
no special inputs.
Instead of approximating how many requests we can still make and hoping
the best that concurrent jobs won't break the bank, we can just work
with the actual limits. By updating regularly, we make sure that
concurrent jobs are taken into account. We always keep a reserve of 1000
requests to make sure *any* non-labeling jobs using this app will always
succeed.
This will allow us to easily backfill labels across multiple days,
especially taking the increased rate limit for the app into account.
With this, we should get up to 11.5k requests per hour in.
This gives us a much higher rate limit of 12.5k requests per hour. If
the app is not available, we fallback to the regular `github.token`.
This can happen when testing in forks without setting up an app.
This will give us a better idea about:
- Which jobs use the most API calls and can possibly be made more
efficient.
- Which rate limits apply exactly to which tokens.
It makes no sense to check cherry picks or request reviewers on PRs from
staging-next to master, the same on backbranches or for haskell-updates
into staging.
Some jobs purposefully only run on certain base or head branches. By
centralizing the logic, parts of it can easily be re-used later. Also,
this gives them an explicit name and thus makes them easier to
understand.
Instead of deleting each label separately and then making another call
to add new labels, this replaces all labels at once, thus saving API
calls in some cases. Also, the labels are now managed in object-style
compared to the array-style before. This allows putting all the
knowledge about each label into a single place instead of in multiple
places. For example, the rebuild labels had to be special cased in the
workflow before - and the nix code to compare had to match that. Also,
the approval labels had to be considered in the `before` and `after`
phases.
The next commit shows how easy it is to add a new label now.
When we made the switch from eval.yml to pr.yml we adjusted the labels
job as well - but didn't take into account that we also need to deal
with old PRs at the same time.
Here, we fallback to another API request to get a run for eval.yml when
we can't find one for pr.yml.
When labels.yml is changed the new code is not tested in the PR
directly, yet, because we forgot to add labels.yml to the list of
pull_request files in pr.yml. This lead to one syntax error merged
already.
First data shows, that we're unlikely to need more than 250 within an
hour of regular activity. Once this is empty, we'll need to wait until
the next hourly refill - thus, we'll rather set this a bit higher to be
on the safe side.
The hourly limit is at 5000 and we peaked around 3500, so far. We'll
certainly have to look into reducing API calls, but this should still
work out for now.
When we switched to a scheduled workflow, we also changed these lines to
take the labels directly from the pull request list we iterate over. At
the time it saved us an API request. Meanwhile, we have introduced
throttling to the workflow and this causes a potential race condition:
When the scheduled or manually triggered workflow is kicked off and
empties its reservoir of API requests it might be blocked to wait up
to.. an hour! If this happens, the labels taken from the pull request
list might already be outdated at the time the workflow continues. This
would cause some labels to be reset to their original state, which could
be wrong if, for example, another push has happened in the meantime.
This will have a much bigger impact after the next commit, where *all*
labels are set every time, thus the `before` part must be accurate.
Fetching the current labels right before managing them reduces this risk
significantly.
This can only happen if the target branch's eval run failed and we're
trying to fix it. In this case, Eval should not fail, even though it
might not be able to do a comparison and add the correct rebuild labels.
But if we failed here, we wouldn't be able to merge the fix once we have
required status checks.
This allows *not* depending on those two jobs with the required status
checks in the next commit, which wouldn't really make sense. If labeling
or pinging maintainers fails for obscure reasons or because the GitHub
API is down, a PR might still pass all other tests and be
merge-eligible.
This fixes a problem where each workflow would get their own merge
commit. This happens frequently when the target branch is merged into a
the same time, different workflows in the same run will run
get-merge-commit at different times and thus have different merge
commits.
Since the jobs don't really depend on each other, this doesn't cause
practical problems, yet. But it has already led to strange CI failures
in a still unmerged PR, which can be prevented from happening with this
clean approach.
And yes, this saves a few API calls on every run.
When the job is run with the pull_request trigger for validation of
changes to the workflow itself, we need to run everything that can be
run without privileges - but not more.
We tried to do so for the three actions/labeler steps, but failed to set
up the condition correctly. We also need to exit early for our
JavaScript based labeler, just before making the mutation requests.
