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

Disable workflows for a forked repository #1932

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sacpis
Copy link
Collaborator

@sacpis sacpis commented Jul 14, 2024

For issue #1852 raised by @schweitzpgi

Test (PR on a forked repository)

@sacpis sacpis force-pushed the disable_workflows_in_forked_repository branch 2 times, most recently from 8759646 to 058f985 Compare July 14, 2024 01:25
@sacpis sacpis marked this pull request as ready for review July 14, 2024 01:27
@sacpis sacpis requested a review from schweitzpgi July 14, 2024 01:27
@sacpis
Copy link
Collaborator Author

sacpis commented Jul 14, 2024

Adding a check for the trigger in CLA job.

Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jul 14, 2024
github-actions bot pushed a commit that referenced this pull request Jul 14, 2024
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

Copy link
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

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

Thanks, Sachin. This looks interesting. Do we need all of these to be disabled or just the ones that automatically run on a schedule? Also, I know I sometimes run the integration_tests.yml from my account, and I'd like to be able to continue to do that as needed. Is there any way to only disable the scheduled runs in forks and retain the ability to run them on demand?

@sacpis
Copy link
Collaborator Author

sacpis commented Jul 15, 2024

Thanks Ben for reviewing this PR.

Here are the scheduled workflows

clean_up
gh_registry
integration_tests
nvqc_regression_tests

Here are the ones which run on a PR in local repository (as can be seen from this PR)

CLA bot
Basic content checks

Do we need all of these to be disabled or just the ones that automatically run on a schedule?

Just to be on the safer side, thought of disabling all. In case, if some tests later get added in the schedule workflows list.

Also, I know I sometimes run the integration_tests.yml from my account, and I'd like to be able to continue to do that as
needed. Is there any way to only disable the scheduled runs in forks and retain the ability to run them on demand?

Yes, there is a way to run any tests in the local PR by removing the condition for forked repo.

@sacpis sacpis force-pushed the disable_workflows_in_forked_repository branch from 097a8f2 to 3272158 Compare July 15, 2024 16:05
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
@sacpis sacpis force-pushed the disable_workflows_in_forked_repository branch from bd2ac6c to d4d97c3 Compare July 15, 2024 20:25
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
@sacpis sacpis enabled auto-merge (squash) July 15, 2024 21:58
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
@bmhowe23
Copy link
Collaborator

Also, I know I sometimes run the integration_tests.yml from my account, and I'd like to be able to continue to do that as
needed. Is there any way to only disable the scheduled runs in forks and retain the ability to run them on demand?

Yes, there is a way to run any tests in the local PR by removing the condition for forked repo.

Maybe I'm in the minority here, but I'd still like to be able to manually run the nightly integration tests from my repo (screenshot below) without having to make my main branch diverge from the upstream main branch by committing a change to revert integration_tests.yml back to how it is right now.

E.g.,
image

Would it be possible to retain this capability by adding structuring the if check like this?

if: ${{ github.event_name == 'workflow_dispatch' || ! github.event.repository.fork }})

I believe this would only need to be done for the ones that have this:

on:
  workflow_dispatch:

We should also get @bettinaheim to weigh in here.

@sacpis sacpis force-pushed the disable_workflows_in_forked_repository branch from c38c280 to a716073 Compare July 16, 2024 15:15
@schweitzpgi
Copy link
Collaborator

Does this check prevent manually running the yml file?

The original issue was logged because these scripts are "on by default". So if you have a fork, you have to go through them all one by one and disable them, or set up email filters to send all the failures to your deleted folder or both. For a fork, these scripts should all be "off by default" since many/most won't have access to the runners, etc. anyway.

@bmhowe23
Copy link
Collaborator

Perhaps relevant: https://github.com/orgs/community/discussions/26704#discussioncomment-5268498

That thread makes it look like users have to manually accept workflows for new forks, at least as of March 2023.

@bmhowe23
Copy link
Collaborator

FWIW, I picked 3 random forks that were opened in the last few months, and they all contain 0 Actions runs:

So maybe GitHub's changes supersede the need for this change?

Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jul 16, 2024
@sacpis
Copy link
Collaborator Author

sacpis commented Jul 25, 2024

@bmhowe23 Should I proceed with the changes as per the suggestions made here?

@bmhowe23
Copy link
Collaborator

@bmhowe23 Should I proceed with the changes as per the suggestions made here?

Perhaps we should discuss at the team meeting. I think that the original issue (#1852) isn't relevant anymore due to GitHub's changes mentioned above. For example, the 3 freshly forked repos I mentioned above have 0 Actions runs, so it seems like the underlying issue won't appear for any new forks.

@sacpis
Copy link
Collaborator Author

sacpis commented Jul 29, 2024

This is @bettinaheim comment on this PR

"A repository can always disable workflows in the GUI:
Personally, I find it a bit impractical if one needs to make a code change to enable the workflows on a fork. If there is a strong preference for that and no complaints, however, I am good with it."
image_720

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.

None yet

3 participants