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

DO NOT CLOSE IT / GitHub workflows / Access Rules #5132

Open
DanilBaibak opened this issue Apr 25, 2024 · 6 comments
Open

DO NOT CLOSE IT / GitHub workflows / Access Rules #5132

DanilBaibak opened this issue Apr 25, 2024 · 6 comments

Comments

@DanilBaibak
Copy link
Contributor

DanilBaibak commented Apr 25, 2024

In the FIRST comment you can specify who will use the linux foundation (LF) runners:

  • Add a GitHub username to use LF runners.
  • Add "*" at the beginning to switch ALL users to LF runners.
  • Add "!" at the beginning to switch ALL users to old runners.
@DanilBaibak
Copy link
Contributor Author

DanilBaibak commented Apr 25, 2024

@DanilBaibak DanilBaibak changed the title ARC dynamic rollout GitHub workflows / Dynamic Rollout May 6, 2024
@DanilBaibak DanilBaibak changed the title GitHub workflows / Dynamic Rollout GitHub workflows / Access Rules May 7, 2024
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this issue May 10, 2024
This PR introduces a tool to dynamically switch between ARC runners and old runners without having to update the PR to the latest version.

There is also a third option - use both runners at the same time (aka shadow deployment). In this case, failed workflows using ARC launchers will not block merge process.

The GitHub issue is used to control access to ARC launchers - [Access Rules Issue](pytorch/test-infra#5132):

* In the FIRST comment you can specify who will use the ARC runners:
   * Add a GitHub username to use ARC runners.
   * Add "*" at the beginning to switch ALL users to ARC runners.
   * Add "!" at the beginning to switch ALL users to old runners.
* In the SECOND comment you can specify do we need to run ARC runners and old runners at the same time.
   * To use both runners, add a second comment with the word "both".
   * If we want to use only one type of runners, just remove the second comment.
Pull Request resolved: #125680
Approved by: https://github.com/ZainRizvi
@kit1980
Copy link
Member

kit1980 commented May 31, 2024

Should we close this?

@DanilBaibak DanilBaibak changed the title GitHub workflows / Access Rules DO NOT CLOSE IT / GitHub workflows / Access Rules Jun 3, 2024
@DanilBaibak
Copy link
Contributor Author

No, pls don't close the issue. It will be used for dynamic rollout while we are migrating from our runners to the linux foundation runners.

pytorchmergebot pushed a commit to pytorch/pytorch that referenced this issue Jun 24, 2024
Enables dynamic migration of jobs to the LF AWS account for the pull workflow.  For now, it leaves out a few jobs that need a bit more testing: Namely Windows and Android runners.

The new runners are only given to people specified in this issue:
pytorch/test-infra#5132

Note: The non-pull jobs updated are the ones that have are synced to jobs in pull.yml (via `sync-tag`) and thus have to be updated whenever their corresponding pull.yml jobs are edited

Based on #128597
Pull Request resolved: #129243
Approved by: https://github.com/zxiiro, https://github.com/huydhn, https://github.com/malfet
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this issue Jun 25, 2024
…129246)

Expect the username in the runner rollover issue (pytorch/test-infra#5132) to be prefixed with a "@".

This will make typos way less likely since github's autocomplete/autoformating will help out

For now, I've updated the issue to have usernames both with and without the @ while this change rolls out

Testing:
Ran the script locally on both this issue and a new test issue and verified they both had the expected output:
```
(venv) (base) ➜  ~/pytorch git:(zainr/improve-get-workflow-type)
python .github/scripts/get_workflow_type.py --github-token github_pat_***  --github-issue 5132 --github-user ZainRizvi --github-branch "zainr/stuff"
{"label_type": "lf.", "message": "LF Workflows are enabled for ZainRizvi. Using LF runners."}
```
Pull Request resolved: #129246
Approved by: https://github.com/zxiiro, https://github.com/huydhn
@malfet
Copy link
Contributor

malfet commented Jun 26, 2024

Removed myself from the list, as LF runners queue is huge.. Also, why everyone need to be mentioned twice?

@zxiiro
Copy link
Contributor

zxiiro commented Jun 26, 2024

Removed myself from the list, as LF runners queue is huge.. Also, why everyone need to be mentioned twice?

PR pytorch/pytorch#129246 changed it so that we are using @ prefixed usernames. So I think the non-@ prefix ones no longer need to be there.

@zxiiro
Copy link
Contributor

zxiiro commented Jun 26, 2024

Looks like pytorch/pytorch#129462 introduced a regression where it no longer properly checked the @ prefixed usernames. I have a fix on that but can't open a PR as it depends on pytorch/pytorch#129500 getting merged first. Just waiting for PR checks to complete and I'll try to get it in and open a PR for the @ fix.

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

4 participants