CI : Dependent Issues

Stream: general

[Jireh Loreaux]

@maintainers: FYI, I got the following error for the dependent issues check on #13719. Just thought someone should know.

Error: HttpError: API rate limit exceeded for user ID 104808330.

[Bryan Gin-ge Chen]

#13902 was merged earlier today which should help address this. Hopefully if you merge master again this shouldn't happen again.

[Eric Wieser]

Do we know the userid of the new bot?

[Scott Morrison]

the new bot's github username is mathlib-dependent-issues-bot, but I don't know how to obtain the userid

[Scott Morrison]

Oh dear. This is the new bot! 104808330

[Scott Morrison]

So our dependent issues bot is hitting the rate limit all by itself. :-(

[Scott Morrison]

Options:

• find a better behaved bot
• write a better behaved bot
• working out how to divide up the work between multiple accounts
• ...?

Help very much appreciated!

[Eric Wieser]

I think the problem is that the bot touches every open PR every time a PR is built? Both seem to grow linearly, so...

[Scott Morrison]

Does anyone know how to get a list of all the "mentions" of a given PR by other PRs? Ideally when an issue is closed, rather than going through all open PRs to see if any depended on it, we could just go through the PRs which have already mentioned it.

[Scott Morrison]

(taking advantage of the fact that github itself has already noticed these mentions when the - [ ] depends on #XXX was written.

[Scott Morrison]

If we can't do this, I can only think of stateful solutions, which sounds unpleasant in a bot.

[Eric Wieser]

The "stateful" solution could be "record backlinks in the PR description of the resolving PR"

[Eric Wieser]

Which at least outsources the state tracking to github.

[Eric Wieser]

But makes the "mentions" idea much worse

[Jireh Loreaux]

I'll note that this happened to me just after I merged master from the other build issue yesterday. Perhaps it was just the case that everyone was doing this at the same time. The API rate limit is hourly, right? So maybe it's not really a problem in general (yet).

[Johan Commelin]

If the bot runs once every 10 minutes, on all PRs, would that solve the issue? Because now, if 15 PRs are merged, it runs 15 times on all PRs. Do I understand that correctly?

[Scott Morrison]

I don't fully understand github actions, but I think our YAML file says that it is running every time a PR is edited.

[Scott Morrison]

And, besides that, every 15 minutes?

[Scott Morrison]

I guess we could just try turning off all the triggers except the 15 minute one and see what happens.

[Johan Commelin]

That's certainly worth a try, I think

[Johan Commelin]

Shouldn't - cron: '15 * * * *' be changed to - cron: '*/15 * * * *' if we want to run every 15 minutes? Otherwise it only runs 15 past every hour. (Which would match the comment on that line.)

[Scott Morrison]

Ah, yes, forgot my cron syntax.

[Gabriel Ebner]

Does anyone know how to get a list of all the "mentions" of a given PR by other PRs? Ideally when an issue is closed, rather than going through all open PRs to see if any depended on it, we could just go through the PRs which have already mentioned it.

Something like this is possible via the GraphQL API (in a single request), I believe. The sync-task-issues action uses this API, and I think it would be fairly straightforward to modify it so that we can use it instead of the dependent issues action. This is what I wrote earlier on the topic:

This action uses the GitHub GraphQL API to find references

Maybe the dependent issues actions should do that as well.

BTW, these actions seem to overlap very much. Isn't the only difference that the dependent issues actions adds/removes labels?

Yeah, I believe we just need to add a line or two here: https://github.com/jonabc/sync-task-issues/blob/8dda495be473d369c79f4161b0279b163933d333/src/sync-task-issues.js#L68

[María Inés de Frutos Fernández]

Jireh Loreaux said:

@maintainers: FYI, I got the following error for the dependent issues check on #13719. Just thought someone should know.

Error: HttpError: API rate limit exceeded for user ID 104808330.

I just got this error too.

[Eric Wieser]

If we make this a cron job rather than an on-pr job, then it won't mess up PR statuses, right?

[Johan Commelin]

What do you mean?

[Eric Wieser]

Right now everything new in the PR list has a red cross by it due to this rate limit

[Eric Wieser]

If we didn't run it as part of the PR suite then this wouldn't happen

[Johan Commelin]

Right

[Johan Commelin]

So that's a win, right?

[Johan Commelin]

#13940 switches to */15 cron

[Johan Commelin]

Please reply with :thumbs_up: or :thumbs_down: if you think this should (not) be merged

[Scott Morrison]

I think we need to get everything unbroken quickly, and can restore functionality later.

[Johan Commelin]

Ok, fair enough, I kicked it on the queue.

[Yaël Dillies]

Wait, for #13940, isn't "every merged PR" much less often than "every 15min"?

[Mauricio Collares]

On average yes, but not worst-case

[Yaël Dillies]

Or is it running the same check several times for each bors batch?

[Mauricio Collares]

I think it's every time a PR/issue is created or edited, as well as every time master changes

[Yaël Dillies]

Ah, that's what I thought too. So the description is erroneous.

[Scott Morrison]

I think the really dangerous case is when bors merges a big batch, and then simultaneously edits all the PRs from that batch, triggering multiple simultaneous runs of the the dependent issues bot.

[Eric Rodriguez]

I'm not sure whether the last commit was meant to have fixed this, but it's still failing on my PRs

[Eric Wieser]

You'll have to merge master still

[Bolton Bailey]

I just got this error on #14017

[Damiano Testa]

@Bolton Bailey this may apply to you too.

[Violeta Hernández]

So what's the fix? Rerun at :00 flat?

[Violeta Hernández]

Because this didn't work

[Violeta Hernández]

#13963 and #14039 are just stuck

[Scott Morrison]

At the moment there is not a fix. We're working on it. :-(

[Violeta Hernández]

Thanks a lot, and no hurries.