-
Notifications
You must be signed in to change notification settings - Fork 20
Enable GCP runner for RHI workflow #370
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
Conversation
aasgaonkar
commented
May 30, 2025
- enable gcp for msvc debug build config
- ignore raytracing failure for GCP runs
76332e0
to
dc9b78b
Compare
@@ -29,14 +29,14 @@ jobs: | |||
# Builds running on self-hosted runners (build + tests) | |||
- { os: windows, platform: "x86_64", compiler: clang, preset: clang, config: "Debug", flags: "unit-test,coverage", runs-on: { group: nvrgfx, labels: [Windows, X64] } } | |||
- { os: windows, platform: "x86_64", compiler: msvc, preset: default, config: "Release", flags: "unit-test", runs-on: { group: nvrgfx, labels: [Windows, X64] } } | |||
- { os: windows, platform: "x86_64", compiler: msvc, preset: default, config: "Debug", flags: "unit-test", runs-on: { group: gcp, labels: [Windows, X64] } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we prefer running on our machines for the time being. So if we want to additionally run on GCP machines, we should probably do that in a separate workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is taking the build only windows workflow and using GCP runner, so it should be a parallel workflow to existing test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true. I was under the impression that we are limited with GCP resources, so I'd rather see these resources spent on testing slang-rhi with latest slang on all platforms/build configs in the slang CI. We have good coverage already with the build machines here. Also we could enable release build testing, should be OK with the amount of build machines we have available now. So, I see this GCP workflow more like a way to test that slang-rhi tests run on GCP. So this could be a dispatch only workflow even.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dryu-nv Thoughts on this? Adding GCP support will add strain on the resources. we have 4 windows runners currently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we spoke about this in the slang scrum meeting - let's merge this and enable GCP platform testing. if there are issues, we can add more windows runeers and make sure there is enough capacity for all.
ac32440
to
60c29a0
Compare
- enable build + test workflow for windows/msvc/debug - remove build only workflow for same config - ignore raytracing failure for GCP runs
60c29a0
to
4c3b894
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
@jkwak-work can you help merge the PR? |
This reverts commit ba0f576.