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

Unable to comprehend handling of stale issue when days-before-issue-stale is -1 #451

Closed
pllim opened this issue May 21, 2021 · 24 comments · Fixed by #540 or C0ZEN/stale#26
Closed
Labels
bug Something isn't working

Comments

@pllim
Copy link

pllim commented May 21, 2021

Describe your issue

What I want: For issues, don't try to apply stale labels, as those are all applied manually by a human. Only go through the issues labeled as stale (manually) and close them after 7 days of inactivity. This action should not try to apply/remove the stale label but only act upon it.

What happened: I ran two configurations (see below) in dry run mode but unsure if any of them have the exact desired outcome from the logs. I wish to fully understand this behavior before production.

Case studies:

  1. [4.2rc1] Test failures on armel: no FPEs astropy/astropy#11011 -- Desired outcome: Close the issue, write a comment, and apply closed-by-bot label.
  2. The following Table formats cannot serialize None astropy/astropy#11710 -- Same outcome as above.

Your stale action configuration

I tried two configurations based on previous discussions with y'all. Relevant snippets below.

First attempt

https://github.com/astropy/astropy/blob/0e5a37485567fc2fac4429c2b4f719ebef6ab397/.github/workflows/ci_cron_daily.yml#L49-L77

jobs:
  stalebot:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/stale@v3
      with:
        debug-only: true

        days-before-issue-stale: -1
        days-before-issue-close: 7
        remove-issue-stale-when-updated: false
        close-issue-message: 'I am going to close this issue because it was marked with **Close?**, but if you feel that this issue should stay open, then feel free to re-open, remove **Close?** and **closed-by-bot** labels, and apply **keep-open** label.'
        close-issue-label: 'closed-by-bot'
        exempt-issue-labels: 'keep-open'
        any-of-issue-labels: 'Close?'

Not sure why it decided it is a no-op. At least, I think this means no-op, as I am looking for a log entry that would say "closing this issue" or something similar but I don't see it. It just says the issues are not stale even though they are.

2021-05-20T03:18:25.6225301Z [#11011] Found this issue last updated at: 2020-11-25T03:58:26Z
2021-05-20T03:18:25.6226383Z [#11011] The option only-labels (​https://github.com/actions/stale#only-labels​) was not specified
2021-05-20T03:18:25.6227424Z [#11011] └── Continuing the process for this issue
2021-05-20T03:18:25.6228463Z [#11011] Days before issue stale: -1
2021-05-20T03:18:25.6229217Z [#11011] The issue is not closed nor locked. Trying to remove the close label...
2021-05-20T03:18:25.6230114Z [#11011] This issue hasn't a stale label
2021-05-20T03:18:25.6231457Z [#11011] The option any-of-labels (​https://github.com/actions/stale#any-of-labels​) was specified to only process the issues and pull requests with one of those labels (1)
2021-05-20T03:18:25.6232768Z [#11011] ├── One of the required labels is present on this issue
2021-05-20T03:18:25.6233670Z [#11011] └── Continuing the process for this issue
2021-05-20T03:18:25.6234284Z [#11011] This issue has no milestone
2021-05-20T03:18:25.6235010Z [#11011] └── Skip the milestones checks
2021-05-20T03:18:25.6243632Z [#11011] This issue has no assignee
2021-05-20T03:18:25.6244572Z [#11011] └── Skip the assignees checks
2021-05-20T03:18:25.6245232Z [#11011] This issue is not stale
2021-05-20T03:18:25.6246363Z [#11011] This issue should be stale based on the last update date the 25-11-2020 (2020-11-25T03:58:26Z)
2021-05-20T03:18:25.6248158Z [#11011] This issue should not be marked as stale based on the option days-before-issue-stale (​https://github.com/actions/stale#days-before-issue-stale​) (-1)
2021-05-20T03:18:28.4571921Z [#11710] Found this issue last updated at: 2021-05-11T13:55:55Z
2021-05-20T03:18:28.4572835Z [#11710] The option only-labels (​https://github.com/actions/stale#only-labels​) was not specified
2021-05-20T03:18:28.4573889Z [#11710] └── Continuing the process for this issue
2021-05-20T03:18:28.4574816Z [#11710] Days before issue stale: -1
2021-05-20T03:18:28.4575541Z [#11710] The issue is not closed nor locked. Trying to remove the close label...
2021-05-20T03:18:28.4576326Z [#11710] This issue hasn't a stale label
2021-05-20T03:18:28.4577997Z [#11710] The option any-of-labels (​https://github.com/actions/stale#any-of-labels​) was specified to only process the issues and pull requests with one of those labels (1)
2021-05-20T03:18:28.4579489Z [#11710] ├── One of the required labels is present on this issue
2021-05-20T03:18:28.4580295Z [#11710] └── Continuing the process for this issue
2021-05-20T03:18:28.4580943Z [#11710] This issue has no milestone
2021-05-20T03:18:28.4581531Z [#11710] └── Skip the milestones checks
2021-05-20T03:18:28.4582109Z [#11710] This issue has no assignee
2021-05-20T03:18:28.4582844Z [#11710] └── Skip the assignees checks
2021-05-20T03:18:28.4583471Z [#11710] This issue is not stale
2021-05-20T03:18:28.4584355Z [#11710] This issue should be stale based on the last update date the 11-05-2021 (2021-05-11T13:55:55Z)
2021-05-20T03:18:28.4585937Z [#11710] This issue should not be marked as stale based on the option days-before-issue-stale (​https://github.com/actions/stale#days-before-issue-stale​) (-1)

Second attempt

Added a stale-issue-label parameter.

https://github.com/astropy/astropy/blob/769031bb61ff8f179e932805d5587d3cdbe798a9/.github/workflows/ci_cron_daily.yml#L49-L79

jobs:
  stalebot:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/stale@v3
      with:
        debug-only: true

        days-before-issue-stale: -1
        days-before-issue-close: 7
        remove-issue-stale-when-updated: false
        close-issue-message: 'I am going to close this issue because it was marked with **Close?**, but if you feel that this issue should stay open, then feel free to re-open, remove **Close?** and **closed-by-bot** labels, and apply **keep-open** label.'
        stale-issue-label: 'Close?'
        close-issue-label: 'closed-by-bot'
        exempt-issue-labels: 'keep-open'
        any-of-issue-labels: 'Close?'

I think the first case is handled correctly, at least in theory.

2021-05-21T03:20:08.4421182Z [#11011] Found this issue last updated at: 2020-11-25T03:58:26Z
2021-05-21T03:20:08.4422019Z [#11011] The option only-labels (​https://github.com/actions/stale#only-labels​) was not specified
2021-05-21T03:20:08.4422801Z [#11011] └── Continuing the process for this issue
2021-05-21T03:20:08.4423351Z [#11011] Days before issue stale: -1
2021-05-21T03:20:08.4423856Z [#11011] The issue is not closed nor locked. Trying to remove the close label...
2021-05-21T03:20:08.4424335Z [#11011] This issue has a stale label
2021-05-21T03:20:08.4425459Z [#11011] The option any-of-labels (​https://github.com/actions/stale#any-of-labels​) was specified to only process the issues and pull requests with one of those labels (1)
2021-05-21T03:20:08.4426721Z [#11011] ├── One of the required labels is present on this issue
2021-05-21T03:20:08.4427393Z [#11011] └── Continuing the process for this issue
2021-05-21T03:20:08.4428001Z [#11011] This issue has no milestone
2021-05-21T03:20:08.4428535Z [#11011] └── Skip the milestones checks
2021-05-21T03:20:08.4428943Z [#11011] This issue has no assignee
2021-05-21T03:20:08.4429461Z [#11011] └── Skip the assignees checks
2021-05-21T03:20:08.4429863Z [#11011] This issue is already stale
2021-05-21T03:20:08.4430304Z [#11011] Checking for label on this issue
2021-05-21T03:20:08.5879512Z [#11011] Issue marked stale on: 2020-11-10T20:16:37Z
2021-05-21T03:20:08.5880177Z [#11011] Checking for comments on issue since: 2020-11-10T20:16:37Z
2021-05-21T03:20:08.6725745Z [#11011] Comments not made by actor or another bot: 0
2021-05-21T03:20:08.6726219Z [#11011] Issue has been commented on: false
2021-05-21T03:20:08.6726661Z [#11011] Days before issue close: 7
2021-05-21T03:20:08.6727044Z [#11011] Issue has been updated: false
2021-05-21T03:20:08.6728069Z [#11011] Closing issue because it was last updated on! 2020-11-25T03:58:26Z
2021-05-21T03:20:08.6728549Z [#11011] Closing issue for being stale
2021-05-21T03:20:08.6728960Z [#11011] 1 operation consumed for this issue

I don't know why it is trying to remove the stale label for the second case. This is not what I thought would happen.

2021-05-21T03:20:03.5529594Z [#11710] Found this issue last updated at: 2021-05-11T13:55:55Z
2021-05-21T03:20:03.5530597Z [#11710] The option only-labels (​https://github.com/actions/stale#only-labels​) was not specified
2021-05-21T03:20:03.5531417Z [#11710] └── Continuing the process for this issue
2021-05-21T03:20:03.5531988Z [#11710] Days before issue stale: -1
2021-05-21T03:20:03.5532488Z [#11710] The issue is not closed nor locked. Trying to remove the close label...
2021-05-21T03:20:03.5532994Z [#11710] This issue has a stale label
2021-05-21T03:20:03.5534051Z [#11710] The option any-of-labels (​https://github.com/actions/stale#any-of-labels​) was specified to only process the issues and pull requests with one of those labels (1)
2021-05-21T03:20:03.5535077Z [#11710] ├── One of the required labels is present on this issue
2021-05-21T03:20:03.5535703Z [#11710] └── Continuing the process for this issue
2021-05-21T03:20:03.5536135Z [#11710] This issue has no milestone
2021-05-21T03:20:03.5536674Z [#11710] └── Skip the milestones checks
2021-05-21T03:20:03.5537175Z [#11710] This issue has no assignee
2021-05-21T03:20:03.5537759Z [#11710] └── Skip the assignees checks
2021-05-21T03:20:03.5538155Z [#11710] This issue is already stale
2021-05-21T03:20:03.5538558Z [#11710] Checking for label on this issue
2021-05-21T03:20:03.5539104Z [#11710] Issue marked stale on: 2021-05-10T16:05:13Z
2021-05-21T03:20:03.5539703Z [#11710] Checking for comments on issue since: 2021-05-10T16:05:13Z
2021-05-21T03:20:03.5540180Z [#11710] Comments not made by actor or another bot: 2
2021-05-21T03:20:03.5540657Z [#11710] Issue has been commented on: true
2021-05-21T03:20:03.5541046Z [#11710] Days before issue close: 7
2021-05-21T03:20:03.5541431Z [#11710] Issue has been updated: false
2021-05-21T03:20:03.5541904Z [#11710] The issue is no longer stale. Removing the stale label...
2021-05-21T03:20:03.5542408Z [#11710] Removing the label "Close?" from this issue...
2021-05-21T03:20:03.5543090Z [#11710] Skipping the process since the issue is now un-stale
2021-05-21T03:20:03.5543591Z [#11710] 1 operation consumed for this issue

Further context

astropy/astropy#11070

@C0ZEN
Copy link
Contributor

C0ZEN commented May 23, 2021

For the first case:
The last log appears when the issue should not be marked as stale.

To be marked as stale, we check that the days before stale are superior or equal to 0.
In your case, it's -1.

If the workflow had mark as stale, it would have considered the issue as stale the like when a stale label was just applied, obviously.
The comment would have been added (if the options allow it).
The stale label would have been added as well.

Since it's -1, it makes sense to have this when working with a cron job for an issue not yet stale.

You mentioned that you wish to have a workflow running with manual stale label, yet we can see in the logs "This issue hasn't a stale label" which mean that currently, the workflow is processing something that is a candidate for the stale action, not the close action.

Checking this issue astropy/astropy#11011 confirm to me that there is no "Stale" label applied which is the default value of the option "stale-issue-label" which is not override by your configuration.

Can you provide the logs of this issue astropy/astropy#11011 but beforehand add a "Stale" label?
Thanks!

@pllim
Copy link
Author

pllim commented May 24, 2021

Re: "Stale" label -- We use "Close?" in lieu of "Stale". I thought this would have made it understand this:

stale-issue-label: 'Close?'
any-of-issue-labels: 'Close?'

https://github.com/astropy/astropy/blob/3f30432d079e6fc07cbcb0d29437f76e33027a1b/.github/workflows/ci_cron_daily.yml#L67

We choose not to create a new label called "Stale" because we have been using "Close?" for many years.

@pllim
Copy link
Author

pllim commented May 24, 2021

Re: logs -- I think these are the ones (the very first and second runs).

First attempt: https://github.com/astropy/astropy/runs/2626331162?check_suite_focus=true

Second attempt: https://github.com/astropy/astropy/runs/2635921679?check_suite_focus=true

@C0ZEN
Copy link
Contributor

C0ZEN commented May 24, 2021

In the first attempt config that you display above, there is no 'stale-issue-label' option.

@pllim
Copy link
Author

pllim commented May 24, 2021

Correct. So I added it in the second attempt and still it didn't quite do what I wanted.

@C0ZEN
Copy link
Contributor

C0ZEN commented May 24, 2021

I will take a look at the second case later

@pllim
Copy link
Author

pllim commented May 24, 2021

Thank you! Your advise would be much appreciated. 🙏

@C0ZEN
Copy link
Contributor

C0ZEN commented May 24, 2021

If the option to remove the stale label when updated is enabled, and if the issue has been commented, then the stale label is removed.
The logs are clearly saying that there is a comment so no issue from this part.
Nonetheless, based on the provided configuration the option remove-issue-stale-when-updated is set to false.
Sadly, there is no log to confirm this however I provided a PR to remediate to this.

FYI, in the code we expect boolean | undefined and if the given value is not a boolean, we use as a fallback the remove-stale-when-updated option which is set by default at 'true'.
There is a parser if the value received is a boolean as string to convert it to boolean.
So basically, I can see based on your information that there is an issue related to this option, but I don't see where, and we can have the proof once my current PR is merged.
If you wish nonetheless to shortcut the process, maybe you can update your workflow config to point to my PR? Never tried this but maybe it's possible! #463

There is also an issue in the default values that I already fixed in this PR #456 because the option remove-issue-stale-when-updated should be unset by default.

@C0ZEN
Copy link
Contributor

C0ZEN commented May 24, 2021

@pllim FYI I have one concern about the configuration.
Basically, we are using under the hood https://www.npmjs.com/package/@actions/core and precisely the method getInput which return a string.
So, I expect the configuration remove-issue-stale-when-updated: false to be parsed as false as a string, not a boolean.
If somehow the TypeScript definition is lying, we have our culprit.
But that seems too big to be true.

@pllim
Copy link
Author

pllim commented May 24, 2021

Yeah, I saw actions/toolkit#725 but that shouldn't have changed getInput. 👀

@pllim
Copy link
Author

pllim commented May 24, 2021

maybe you can update your workflow config to point to my PR

I think that is possible but I might not get to it for a few days. Thanks for all your help!

@pllim
Copy link
Author

pllim commented May 26, 2021

Actually thinking about this more, testing this live on our repo downstream might be a little controversial, as it will introduce a bunch of temporary commits into main itself, and I am only one of the many maintainers there.

Is there a way to generate the log you want locally? Say, use some GitHub API to pull in the astropy issues metadata and then run this Action locally somehow?

@C0ZEN
Copy link
Contributor

C0ZEN commented May 26, 2021

If you wait for the merge of #463, and update your config to use the main branch, you can enable the debug logs and the debug-only option as mentioned in the documentation.
No operation will be executed on your repository and the logs will still be visible.

@pllim
Copy link
Author

pllim commented May 26, 2021

Yes, it is currently running on v3 in debug mode, so switching to main would be less controversial maybe.

@C0ZEN
Copy link
Contributor

C0ZEN commented Jun 1, 2021

@pllim I found an issue that I created...
It's comforting me in the decision to increase the quantity of logs and give transparency over which option cause what with what kind of value behind it because in this case, the culprit would have been identified quite sooner.

So, here is the mistake:

core.getInput('remove-issue-stale-when-updated')

As you can, you see, instead of giving to _toOptionalBoolean the name of the input, it get the value...
I forgot that internally, the function already get the input so the value at the end given to this option was not the right one...
Sorry!

C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 1, 2021
@pllim
Copy link
Author

pllim commented Jun 1, 2021

No worries. I am glad you found the cause. Thank you for your continued help!

C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 3, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 7, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 8, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 10, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 14, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 18, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Jun 24, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2021

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the Stale label Jul 2, 2021
@C0ZEN
Copy link
Contributor

C0ZEN commented Jul 2, 2021

up

@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the Stale label Aug 18, 2021
@C0ZEN
Copy link
Contributor

C0ZEN commented Aug 18, 2021

up

@github-actions github-actions bot removed the Stale label Aug 19, 2021
@pllim
Copy link
Author

pllim commented Aug 31, 2021

Thanks! After some contemplation, we decided to port our old bot instead due to the differences between our stale policies and yours. Still, I am looking forward to use this for a simpler project.

@pllim pllim closed this as completed Aug 31, 2021
@deivid-rodriguez
Copy link

It seems though that there was an intention to keep digging into this? I was looking into using this action, but I have the same policy as described in this ticket, so this issue being fixed would help me :)

@pllim pllim reopened this Aug 31, 2021
@pllim
Copy link
Author

pllim commented Aug 31, 2021

Ok, I'll reopen then. :)

@pllim
Copy link
Author

pllim commented Aug 31, 2021

For academic purpose, I share with you the logic we decided to go with. If you end up using it, please be advised that I wrote it for a particular project in mind and probably won't try to generalize it for other policies: https://github.com/pllim/action-astropy-stalebot

C0ZEN added a commit to C0ZEN/stale that referenced this issue Sep 14, 2021
C0ZEN added a commit to C0ZEN/stale that referenced this issue Sep 14, 2021
luketomlinson pushed a commit that referenced this issue Sep 17, 2021
…ity (#540)

* chore(assignees): add logs

* docs(readme): use the override syntax to simplify the reading

* docs(readme): add missing default options

* docs(readme): add 3 new options to ignore activity before stale

* chore(action): add 3 new options

* fix(removeStaleWhenUpdated): use the value of the action config as expected

Fixes #451

* chore(main): add 3 new options

* feat(ignore): add new class to ignore all activities before stale

* feat(option): add new options to ignore all activities before stale

* chore(index): update index file

* docs(readme): fix typo

* docs(readme): add missing empty row

* chore(rebase): fix logger issues due to rebase

* chore: aplly changes due to rebase

* refactor(naming): change the name of the options as suggested

* chore(logs): reverse the logs as well

* docs(readme): format the table of options

* refactor(naming): rename the the options

* style(rename): rename more updates wording to activities

* build(ci): run the test step as expected for a CI

instead of using a real linter with auto fix and the tests verbose as fuck

* chore: handle breaking changes due to new changes

* refactor(naming): rename and reverse the options

* style(tests): use plural for some describe

* docs(days-before-stale): list the new option

* chore(index): update index file

* chore: keep static methods on top

* chore(logs): remove useless log
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment