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

Commit skipped because of previously skipped duplicate #90

Closed
fmigneault opened this issue Feb 24, 2021 · 15 comments
Closed

Commit skipped because of previously skipped duplicate #90

fmigneault opened this issue Feb 24, 2021 · 15 comments
Labels
documentation Improvements or additions to documentation

Comments

@fmigneault
Copy link

fmigneault commented Feb 24, 2021

I got this workflow:
https://github.com/crim-ca/weaver/blob/master/.github/workflows/tests.yml

On one later commit in the PR, the check produces:
https://github.com/crim-ca/weaver/pull/219/checks?check_run_id=1973904249

Run fkirc/skip-duplicate-actions@master
  with:
    concurrent_skipping: same_content
    skip_after_successful_duplicate: true
    do_not_skip: ["pull_request", "workflow_dispatch", "schedule", "release"]
    github_token: ***
    paths_ignore: []
    paths: []
    cancel_others: true
Did not find other workflow-runs to be cancelled
Skip execution because the exact same files are concurrently checked in https://github.com/crim-ca/weaver/actions/runs/597418175

And that referenced run https://github.com/crim-ca/weaver/actions/runs/597418175 has:

image

So basically, the code was never actually tested.
Is there a way to avoid this issue? Is there something incorrectly configured in the workflow?
https://github.com/crim-ca/weaver/blob/wps-load-cfg/.github/workflows/tests.yml

I have another repo that has similar definition, but somehow I'm not facing the same issues:
https://github.com/Ouranosinc/Magpie/blob/master/.github/workflows/tests.yml

I'm a bit confused by what might be wrong in this case.

@hugoh
Copy link
Contributor

hugoh commented May 24, 2021

This just happened to me as well. There seems to be a race condition in checking for duplicates.

In my case:

  • Workflow 1 running at URL1 said: Skip execution because the exact same files are concurrently checked in URL2
  • Workflow 2 running at URL2 said: Skip execution because the exact same files are concurrently checked in URL1

So, both workflows stopped, and neither fully ran.

@fkirc
Copy link
Owner

fkirc commented May 24, 2021

I suspect that you might need an additional config like do_not_skip: ["pull_request", "workflow_dispatch", "schedule"] or something similar.
In theory, the do_not_skip-config should be used to specify which concurrent runs should not be skipped.

@hugoh
Copy link
Contributor

hugoh commented May 24, 2021

@fkirc - I think the problem is that all runs calling detectConcurrentRuns() reach the conclusion that there is a same_content run and bail out. 1 of them should continue. See #112 as an (untested) proposal to keep the oldest one running. Hopefully I didn't butcher the logic here.

@fmigneault
Copy link
Author

@hugoh @fkirc
I gave a try to the new same_content_newer from #112
I'm not sure why my runs are still being skipped, although they both clearly print out they should not skip. Something seems to still behave incorrectly.

push: https://github.com/bird-house/twitcher/pull/106/checks?check_run_id=3447659820

Run fkirc/skip-duplicate-actions@master
  with:
    concurrent_skipping: same_content_newer
    skip_after_successful_duplicate: true
    do_not_skip: ["pull_request", "workflow_dispatch", "schedule", "release"]
    github_token: ***
    paths_ignore: []
    paths: []
    cancel_others: false
Did not find any skippable concurrent workflow-runs
Do not skip execution because we did not find a transferable run

pull_request: https://github.com/bird-house/twitcher/pull/106/checks?check_run_id=3447660041

Run fkirc/skip-duplicate-actions@master
  with:
    concurrent_skipping: same_content_newer
    skip_after_successful_duplicate: true
    do_not_skip: ["pull_request", "workflow_dispatch", "schedule", "release"]
    github_token: ***
    paths_ignore: []
    paths: []
    cancel_others: false
Do not skip execution because the workflow was triggered with 'pull_request'

@dfuchss
Copy link

dfuchss commented Jun 24, 2022

Are there any solutions for this behavior. We wanted use this action to prevent the duplicate build of "pull_requests" and "branches" for PRs we open in our own repositories :)

@dfuchss
Copy link

dfuchss commented Jun 24, 2022

Maybe you could use something like this process:

  1. Get all concurrent runs that are running and not skipped.
  2. Order them by type/name/something unique (run_id if something like this exist)
  3. Cancel only if the run is not the first in this sorted list of run

@paescuj
Copy link
Collaborator

paescuj commented Jun 24, 2022

The reported issue is quite old now and I've never experienced it on my own. Since the code base has been updated in the meantime quite a bit, perhaps the problem really no longer exists.

@dfuchss How about just giving it a try? In case of any problems feel free to report it here and I'll take a closer look at it.
Please make sure to use the same_content_newer option as stated above.

@fmigneault I know it has been a long time, but do you think you could give it another try? Unfortunately, the logs of your workflows are no longer available, so I wasn't able to locate the problem you had at this point.

@dfuchss
Copy link

dfuchss commented Jun 24, 2022

@paescuj thanks for the fast response. I'll give same_content_newer a try :)

@dfuchss
Copy link

dfuchss commented Jun 24, 2022

Thanks for your help! For our repositories same_content_newer resolves the issue.

@paescuj
Copy link
Collaborator

paescuj commented Jun 24, 2022

@dfuchss Thank you for your fast response as well 😃 I'm glad that it seems to work!

@paescuj paescuj added the documentation Improvements or additions to documentation label Jun 27, 2022
@fmigneault
Copy link
Author

I've left the workflow with those options:
https://github.com/crim-ca/weaver/blob/master/.github/workflows/tests.yml#L19-L23

So far I've not seen the concurrent-skip happen again. Probably fixed from previous PR merged in master.
I think we can consider this resolved. I will open again if it occurs again.

@paescuj
Copy link
Collaborator

paescuj commented Jul 18, 2022

@fmigneault Thanks for your feedback!

@fmigneault
Copy link
Author

fmigneault commented Mar 27, 2024

@paescuj
Sorry to bring back this old issue. It seems I have still the same problem as before. I tried also with same_content_newer without luck.

The following run https://github.com/stac-extensions/ml-aoi/actions/runs/8458356693?pr=8 shows that tests job was skipped because of skip_duplicate, although the summary clearly shows it shouldn't.
image

Going into the skip_duplicate job step itself, we see the following:
https://github.com/stac-extensions/ml-aoi/actions/runs/8458356693/job/23172403701?pr=8
image

So, for some reason, the "do not skip" is evaluated properly, but output is still set to should_skip?

Please let me know what could be wrong in the definition:
https://github.com/stac-extensions/ml-aoi/blob/pystac-pydantic-ext-impl/.github/workflows/test.yaml

@fmigneault fmigneault reopened this Mar 27, 2024
@fmigneault
Copy link
Author

😅 Didn't take long to figure out.
Self note for when I encounter this again.
The issue was the wrong reference in the check when setting the output:

stac-extensions/ml-aoi@1b52721 (#8)

@paescuj
Copy link
Collaborator

paescuj commented Mar 27, 2024

@fmigneault Great, thanks for the update 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

5 participants