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

ci: Restore rosetta-t5x-test #749

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

Conversation

ko3n1g
Copy link
Contributor

@ko3n1g ko3n1g commented Apr 22, 2024

We noticed together that .github/workflows/_test_rosetta.yaml wasn't used anymore. Suggestion by @terrykong was to consolidate it together with .github/workflows/_test_rosetta_t5x.yaml

@@ -6,34 +6,33 @@ on:
T5X_IMAGE:
type: string
description: T5X image from ghcr.io/nvidia/t5x
default: 'ghcr.io/nvidia/t5x:latest'
Copy link
Collaborator

Choose a reason for hiding this comment

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

To my understanding, single quotes are preferred in GitHub Actions as they are less likely to cause unintended effects (not 100% sure how much practical impact it will bring though). Besides, it is better to be consistent across code base 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I changed my formatter's default to single-quotes

@ko3n1g ko3n1g force-pushed the ko3n1g/ci/restore-rosetta-t5x-test branch from 1d15a60 to 80ccc49 Compare April 22, 2024 14:49
@ko3n1g ko3n1g marked this pull request as ready for review April 22, 2024 14:53
@ko3n1g ko3n1g requested a review from yhtang April 22, 2024 14:53
@yhtang
Copy link
Collaborator

yhtang commented Apr 23, 2024

Hitting this GHA limitation agaaaaaaain 🤣

[Invalid workflow file: .github/workflows/ci.yaml#L1](https://github.com/NVIDIA/JAX-Toolbox/actions/runs/8795370520/workflow)
too many workflows are referenced, total: 21, limit: 20

@ko3n1g
Copy link
Contributor Author

ko3n1g commented Apr 23, 2024

Hitting this GHA limitation agaaaaaaain 🤣

[Invalid workflow file: .github/workflows/ci.yaml#L1](https://github.com/NVIDIA/JAX-Toolbox/actions/runs/8795370520/workflow)
too many workflows are referenced, total: 21, limit: 20

Lol, that's really wild. Seems we found the reason why it was left like that? How important is this test?

What do you think about converting some workflows to actions? I'm looking at those workflows which consist of a single job only. These are the pros/cons I can see:

  • (+) Less likely to hit the workflow quota
  • (-) Tasks that are currently rendered as multiple steps will then be condensed into a single step. This also affects the ability to re-run a specific step of a job, which won't be possible anymore once we convert single-job workflows to actions

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.

2 participants