Skip to content

Conversation

@ebr
Copy link
Contributor

@ebr ebr commented Mar 9, 2023

I found it to be a chore to remove labels manually in order to "un-stale" issues. This is contrary to the bot message which says commenting should remove "stale" status. On the current cron schedule, there may be a delay of up to 24 hours before the label is removed. This PR will trigger the workflow on issue comments in addition to the schedule.

Also adds a condition to not run this job on PRs (Github treats issues and PRs equivalently in this respect), and rewords the messages for clarity.

@ebr ebr requested review from lstein and mauwii as code owners March 9, 2023 14:30
Copy link
Collaborator

@lstein lstein left a comment

Choose a reason for hiding this comment

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

LGTM

@lstein lstein merged commit a0065da into main Mar 10, 2023
@lstein lstein deleted the unstale-issues branch March 10, 2023 01:17
@mauwii
Copy link
Contributor

mauwii commented Mar 10, 2023

Triggering the action by comment isn't a good idea, since it is processing the same issues as with the cronjob, which could lead to run into the rate limit if to many comments are done:

image

Screenshots are from those two runs:
https://github.com/invoke-ai/InvokeAI/actions/runs/4380443015/jobs/7667515322
https://github.com/invoke-ai/InvokeAI/actions/runs/4380477965/jobs/7667590261

@lstein lstein restored the unstale-issues branch March 10, 2023 03:24
lstein added a commit that referenced this pull request Mar 10, 2023
Reverts #2903

@mauwii has a point here. It looks like triggering on a comment results
in an action for each of the stale issues, even ones that have been
previously dealt with. I'd like to revert this back to the original
behavior of running once every time the cron job executes.

What's the original motivation for having more frequent labeling of the
issues?
@ebr
Copy link
Contributor Author

ebr commented Mar 12, 2023

Yes, I was fully aware of the number of issues being processed with this change.

Does it actually lead to hitting any rate limits, or is this a hypothetical?

@mauwii can you suggest an improvement to this by getting it to process only specific issues, instead of rejecting it outright?

@mauwii
Copy link
Contributor

mauwii commented Mar 12, 2023

With the latest change, the limit would already be hit with two comments to issues / hour (which would already have been done).

And since the 500 operations-per-run still not process all of the issues, I am not sure if it needs to be raised again.

I also had no chance to suggest anything since it was merged before I even saw the PR ...

@lstein lstein deleted the unstale-issues branch April 11, 2023 15:14
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.

4 participants