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

Move to pandas-tests to a dedicated workflow file and trigger it from branch.yaml #15516

Merged
merged 13 commits into from
Apr 12, 2024

Conversation

galipremsagar
Copy link
Contributor

Description

This PR moves pandas-tests to a dedicated workflow file and trigger's it from branch.yaml

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@galipremsagar galipremsagar added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 11, 2024
@galipremsagar galipremsagar self-assigned this Apr 11, 2024
@galipremsagar galipremsagar requested a review from a team as a code owner April 11, 2024 20:04
@github-actions github-actions bot added the ci label Apr 11, 2024
.github/workflows/build.yaml Outdated Show resolved Hide resolved
Copy link
Member

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.

Copy link
Member

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

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" ))
Copy link
Member

@ajschmidt8 ajschmidt8 Apr 11, 2024

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.

Copy link
Contributor Author

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' }

https://github.com/rapidsai/shared-workflows/blob/13e008c746e30a830c4ebe83a6786862858dfcb8/.github/workflows/wheels-test.yaml#L75-L94`

Copy link
Contributor Author

@galipremsagar galipremsagar Apr 11, 2024

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)

Copy link
Contributor Author

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.

Copy link
Contributor

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)
Copy link
Contributor Author

@galipremsagar galipremsagar Apr 11, 2024

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.

@ajschmidt8 ajschmidt8 dismissed their stale review April 12, 2024 16:39

There is no obvious solution to the issue I raised, so dismissing my change request for now

@galipremsagar
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 6f8ff79 into rapidsai:branch-24.06 Apr 12, 2024
71 checks passed
@galipremsagar galipremsagar mentioned this pull request Apr 13, 2024
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Apr 15, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants