-
Notifications
You must be signed in to change notification settings - Fork 875
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
Node/npm requirement for Graphs CI workflow #812
Comments
@houserx-jmcc Maybe it's best to add this to the documentation instead (would love to accept a PR for that!)? I think we can add a note that if you're using as self-hosted runner, make sure running If you feel differently, I'm happy to reopen this issue and discuss! |
This is a bit easier said than done (at least in my case). We'd have to start packaging our own runner images and get a pipeline setup to keep them up to date with the upstream base version, just for a single Node-related command in a single project. We utilize GitHub's actions/setup-node action in almost every workflow, and its honestly quite fast since they do a lot of caching behind the scenes. Its almost always 0-2 seconds, and includes the npm tools with it. Using it would also allow you more flexibility for Node minor versions and updates. With the additional lift to build out custom runner image, it would probably be a deal breaker for us to invest in using this unfortunately. A compromise might be to add the step and let it be toggleable with an environment variable, so it would be skipped by default but enabled for those who need it. |
Describe the bug
When using self-hosted runners (which tend to be slimmer images than the GitHub hosted runners) the Graphs CI workflow fails due to its reliance on npx and it not existing.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
The workflow should be able to work, even on slim images.
Screenshots
I think the most straightforward fix would be to add the actions/setup-node action as a precursor step in the Graphs CI workflow to ensure the dependency is always met. I am happy to make a PR for this.
The text was updated successfully, but these errors were encountered: