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

Update node16 check on self-hosted runners and remove python 3.6 #5756

Merged
merged 4 commits into from
Jul 8, 2024

Conversation

loadams
Copy link
Contributor

@loadams loadams commented Jul 8, 2024

With changes from GitHub finally deprecating node16 based runners (which the checkout@v3 action uses) we need to make changes to support this.

To do this, there are two changes. First we remove the python 3.6 check as with the changes in pydantic v2 that will be merged soon, we will be removing this check there, so we can more easily remove it now so that future PRs are cleaner and it is clear why some changes have been made.

Second, node16 is the default on some of our self-hosted runners. To work around tests failing on these, we set the GitHub env var to override this check.

Other relevant links:
actions/checkout#1474
https://github.com/easybuilders/easybuild-framework/pull/4574/files
actions/checkout#1809
actions/runner#3373
actions/checkout#1809

@loadams loadams changed the title Test with env var set Update node16 check on self-hosted runners and remove python 3.6 Jul 8, 2024
@loadams
Copy link
Contributor Author

loadams commented Jul 8, 2024

Merged #5755 into this PR to have a single PR that fixes both issues.

@loadams loadams requested review from adk9 and tjruwase July 8, 2024 17:54
@loadams
Copy link
Contributor Author

loadams commented Jul 8, 2024

Workflow actions for nv-lightning and python are tested here, linked below are builds for nv-torch110-v100 and nv-torch110-p40 (both are now at least getting past this error and hitting their other current errors):

https://github.com/microsoft/DeepSpeed/actions/runs/9845125845
https://github.com/microsoft/DeepSpeed/actions/runs/9845127511

@loadams
Copy link
Contributor Author

loadams commented Jul 8, 2024

nv-torch-latest-v100 is failing and known, bypassing to get these fixes merged.

@loadams loadams merged commit 8411816 into master Jul 8, 2024
10 of 13 checks passed
@loadams loadams deleted the loadams/fix-node16-deprecation-runners branch July 8, 2024 19:33
mauryaavinash95 pushed a commit to DataStates/DeepSpeed that referenced this pull request Jul 10, 2024
…rosoft#5756)

With changes from GitHub [finally
deprecating](actions/checkout#1474) [node16
based
runners](https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/)
(which the checkout@v3 action uses) we need to make changes to support
this.

To do this, there are two changes. First we remove the python 3.6 check
as with the changes in pydantic v2 that will be merged soon, we will be
removing this check there, so we can more easily remove it now so that
future PRs are cleaner and it is clear why some changes have been made.

Second, node16 is the default on some of our self-hosted runners. To
work around tests failing on these, we [set the GitHub env var to
override this
check](https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/).

Other relevant links:
actions/checkout#1474
https://github.com/easybuilders/easybuild-framework/pull/4574/files
actions/checkout#1809
actions/runner#3373
actions/checkout#1809
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

2 participants