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

Action produces hundreds of failed builds #27

Open
asmeurer opened this issue Jul 8, 2022 · 20 comments
Open

Action produces hundreds of failed builds #27

asmeurer opened this issue Jul 8, 2022 · 20 comments

Comments

@asmeurer
Copy link
Contributor

asmeurer commented Jul 8, 2022

If you look at https://github.com/sympy/sympy/actions/workflows/docs-preview.yml, you can see there are a ton of failed builds for the docs preview action. I think this is because it runs every time there is a status change from any CI job, but it only makes sense when the status change actually comes from the CircleCI job.

Is it possible to make the action not run at all in these cases? It's obviously not a huge issue, but the current way is fairly confusing, and makes the "all workflows" list harder to read https://github.com/sympy/sympy/actions.

By the way, do you know why half of those jobs are indicated as being started by me, even for commits that I had nothing to do with? How does GitHub determine who the author of a "status" update is?

@larsoner
Copy link
Collaborator

larsoner commented Jul 9, 2022

Is it possible to make the action not run at all in these cases?

Not sure, someone will have to look at the GitHub actions docs to see if the on: status can be made more selective in YAML, or if it has to short-circuit in the action itself.

If we can't make the YAML more selective (which is what I'd expect), we should at least make it exit gracefully rather than emit an error.

How does GitHub determine who the author of a "status" update is?

No idea...

@asmeurer
Copy link
Contributor Author

It seems like if could be used to do this, if I am reading https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#status correctly.

However, I tried it at sympy/sympy#23801 and I can't get it to work. Any suggestions? It's super annoying to test this since it has to be merged into master before it actually does anything, so I'd prefer to minimize the number of iterations here. Or if you have a better way to test the action out that would be helpful.

@larsoner
Copy link
Collaborator

It's super annoying to test this since it has to be merged into master before it actually does anything, so I'd prefer to minimize the number of iterations here.

I would test it out by iterating by cycling through two steps

  1. Committing directly to the master branch of your fork of this repository some changeset you think should work
  2. Pushing an empty commit to a dummy branch in your fork of this repository, with a PR open to merge dummy into master

Every dummy commit you push in step (2) should immediatly allow you to see if step (1) had the effect you wanted.

Once it does, you can open a PR to merge your master branch into the upstream one here (with or without cleaning up the commits first -- I'll just squash + merge so it won't make much difference)

@rgommers
Copy link

I was looking into this as well. I think on: [status] is the wrong trigger, and it cannot be made more specific later on. The right one seems to be repository_dispatch.

See also https://stackoverflow.com/questions/67330220/trigger-github-action-after-circleci-workflow-or-job.

@larsoner
Copy link
Collaborator

I think on: [status] is the wrong trigger,

In some sense yes, because it has to look at all status events to find the right one. In another sense no, in that it does work without needing to make any changes to your CircleCI config...

The right one seems to be repository_dispatch.

... unlike this solution. I think this would require anyone who wanted to use this solution to add such a dispatch to their CircleCI job.

So I think we could advertise two ways to do it:

  1. The current ugly way that leads to a lot of extra logging and runs, but works.
  2. A better way that uses repo dispatch

@rgommers I think you might be able to try the repo dispatch approach (2) on SciPy already by modifying the CircleCI job to dispatch the event, and modify the circleci-artifacts-redirector-action YAML to accept it, right?

@larsoner
Copy link
Collaborator

I might give this a shot in this repo. It'll be easier and way less noisy than trying it in SciPy :)

@larsoner
Copy link
Collaborator

I think this might not be tractable. To launch a GitHub repo event, whatever emits/posts the repository_dispatch event must have write access to the repo:

https://docs.github.com/en/rest/repos/repos#create-a-repository-dispatch-event

You can do this via:

  1. A GitHub app with the correct permissions, but CircleCI is not an app, or
  2. A personal access token with repo-level permissions

For (2) to work, a maintainer would need to create a personal access token with access to the repo, put this in the CircleCI config, and make this visible/accessible/usable by forked PRs, which is unsafe because anybody could in the CircleCI job echo or post this token somewhere else and thereby gain repo-level permissions. I think that's the case anyway...

@rgommers
Copy link

Argh, that's not nice. Agreed that this sounds too risky. Thanks for looking into it!

@asmeurer
Copy link
Contributor Author

It still might be possible to make this work with if. I couldn't figure it out, but the docs certainly seem to imply that it ought to work.

@larsoner
Copy link
Collaborator

Maybe with #32 ?

@asmeurer
Copy link
Contributor Author

Maybe we should open a feedback issue about this at https://github.com/community/community/discussions/categories/general-feedback

@bsipocz
Copy link
Member

bsipocz commented Apr 20, 2023

Now that we start using access tokens anyway, I wonder whether this could be fixed, too? #27 (comment)

@larsoner
Copy link
Collaborator

I am not optimistic because of this part of that comment:

and make this [token] visible/accessible/usable by forked PRs, which is unsafe because anybody could in the CircleCI job echo or post this token somewhere else and thereby gain repo-level permissions. I think that's the case anyway...

@bsipocz
Copy link
Member

bsipocz commented Apr 20, 2023

Ahh, I was way too enthusiastic than a careful reader then.

@bsipocz
Copy link
Member

bsipocz commented Apr 20, 2023

To follow-up: I could skip running the workflow for non-circleCI related jobs with a conditional in jobs:, but they are still listed as cancelled ones in the actions list (I couldn't make a conditional work in the on: section).

I also tried to use check_run, but couldn't manage to trigger the workflow with that one, and even if it worked I believe it would not be the right approach either as it totally disentangles from the circleCI status and trigger only once all CI checks have finished/succeded.

@pllim
Copy link
Contributor

pllim commented Sep 16, 2024

I just ran into this (though I got 4 jobs instead of hundreds?) but the end result is the same (no preview). Any workaround I can use? Thanks!

@bsipocz
Copy link
Member

bsipocz commented Sep 16, 2024

@pllim - having stalling jobs, I wonder if the tokens are rotated properly? Note that I run into the multiple triggers, but haven't yet seen what you run into yet. (I certainly saw #58, but as I recall those were around only at the time when I was setting up a repo so not everything were at their right place).

@larsoner
Copy link
Collaborator

Keep in mind GHA is having a bad day https://www.githubstatus.com/

Screenshot 2024-09-16 at 6 03 48 PM

@pllim

This comment was marked as resolved.

@pllim
Copy link
Contributor

pllim commented Sep 17, 2024

For completeness: #59 (comment)

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

No branches or pull requests

5 participants