-
Notifications
You must be signed in to change notification settings - Fork 912
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
Move to pandas-tests to a dedicated workflow file and trigger it from branch.yaml #15516
Move to pandas-tests to a dedicated workflow file and trigger it from branch.yaml #15516
Conversation
.github/workflows/pandas-tests.yml
Outdated
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.
let's make this file extension .yaml
to be consistent with (most of) the others.
the java workflow is the outlier and that should probably be updated to, though I don't expect you to do that in this PR.
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.
don't forget to update build.yaml
too when making this change
Co-authored-by: AJ Schmidt <[email protected]>
…ar/cudf into move_pandas_test_job
Co-authored-by: Ray Douglass <[email protected]>
secrets: inherit | ||
uses: rapidsai/shared-workflows/.github/workflows/[email protected] | ||
with: | ||
matrix_filter: map(select(.ARCH == "amd64" and .PY_VER == "3.9" and .CUDA_VER == "12.2.2" )) |
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.
Hardcoding matrix filter values is error prone since they can fail as our support matrix changes.
What's the intended matrix that this should run against? The latest CUDA version and earliest Python version?
Once I know that information, I can help you put together a more generalized matrix filter.
cc: @bdice for additional input.
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 because the baseline job is being run as nightly
job and comparison job is being run as a pull-request
job. The only matrix combination common to both is
- { ARCH: 'amd64', PY_VER: '3.9', CUDA_VER: '12.2.2', LINUX_VER: 'ubuntu22.04', gpu: 'v100', driver: 'latest' }
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.
@bdice too pointed out this inconsistency before, but I haven't found a way to write a matrix filter that will fetch an identical job for both the jobs of pandas-tests
in pr.yaml
(pull-request build type) and branch.yaml
(nightly build type)
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.
Using different job configurations is not an option here because that will create some noise(test failures) that's specific to certain configurations.
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 kind of thing is why I've advocated for the nightly matrix to be a strict superset of the PR matrix.
rapidsai/shared-workflows#176 (comment)
rapidsai/build-planning#5
@@ -8,14 +8,16 @@ | |||
|
|||
# Hard-coded needs to match the version deduced by rapids-upload-artifacts-dir | |||
GH_JOB_NAME="pandas-tests-diff / build" | |||
RAPIDS_FULL_VERSION=$(<./VERSION) |
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.
@raydouglass Instead of introducing a hard-coded version, I'm re-utilizing VERSION
value that is more than enough for us here thus not needing to update the version script entry.
There is no obvious solution to the issue I raised, so dismissing my change request for now
/merge |
This PR removes version hard-coding introduced in #15516 Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Bradley Dice (https://github.com/bdice) - Ray Douglass (https://github.com/raydouglass) URL: #15529
Description
This PR moves pandas-tests to a dedicated workflow file and trigger's it from
branch.yaml
Checklist