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

Enable test-reporting for pandas pytests in CI #15369

Merged
merged 98 commits into from
Apr 8, 2024

Conversation

galipremsagar
Copy link
Contributor

@galipremsagar galipremsagar commented Mar 21, 2024

Description

This PR enables pandas test-reporting for pandas pytests in CI by comparing against the results available in nightlies as a baseline.

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 Mar 21, 2024
@galipremsagar galipremsagar self-assigned this Mar 21, 2024
@github-actions github-actions bot added the ci label Mar 21, 2024
Comment on lines 10 to +11
MAIN_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py310.main-results.json
PR_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py310.pr-results.json
aws s3 cp $MAIN_ARTIFACT main-results.json
PR_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py39.pr-results.json
Copy link
Member

Choose a reason for hiding this comment

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

The python version changed and is different from the main artifact. Is that expected?

Also, why not use Python 3.11?

Copy link
Contributor Author

@galipremsagar galipremsagar Apr 8, 2024

Choose a reason for hiding this comment

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

answered here: #15485 (comment)

There seems to be no python 3.11 for cuda-12.2 pull-request build type.

Comment on lines 10 to +11
MAIN_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py310.main-results.json
PR_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py310.pr-results.json
aws s3 cp $MAIN_ARTIFACT main-results.json
PR_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py39.pr-results.json
Copy link
Member

Choose a reason for hiding this comment

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

It would be better if these versions were not hard coded if possible.

If that can't be accomplished easily, please file an issue for it but extract the values to variables defined prominently in the beginning of the file.

Copy link
Contributor Author

@galipremsagar galipremsagar Apr 8, 2024

Choose a reason for hiding this comment

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

I don't think that would still work here, the actual job that generates the json files is different from the job that runs this file, so we wouldn't be able to utilize the machine's versions here.


rapids-logger "Fetching latest available results from nightly"
aws s3api list-objects-v2 --bucket rapids-downloads --prefix "nightly/" --query "sort_by(Contents[?ends_with(Key, '.main-results.json')], &LastModified)[::-1].[Key]" --output text > s3_output.txt
cat s3_output.txt
Copy link
Member

Choose a reason for hiding this comment

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

This is printed by the logger on line 18

PR_ARTIFACT=$(rapids-s3-path)cuda12_$(arch)_py39.pr-results.json

rapids-logger "Fetching latest available results from nightly"
aws s3api list-objects-v2 --bucket rapids-downloads --prefix "nightly/" --query "sort_by(Contents[?ends_with(Key, '.main-results.json')], &LastModified)[::-1].[Key]" --output text > s3_output.txt
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this logic should be moved to gha-tools since it relies on the paths created there.

ci/cudf_pandas_scripts/pandas-tests/diff.sh Outdated Show resolved Hide resolved
Co-authored-by: Ray Douglass <3107146+raydouglass@users.noreply.github.com>
Copy link
Member

@raydouglass raydouglass left a comment

Choose a reason for hiding this comment

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

Talked with @galipremsagar and I will approve to unblock things, but Prem will address some of my feedback in a follow up PR.

@galipremsagar
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 102d564 into rapidsai:branch-24.06 Apr 8, 2024
73 of 74 checks passed
@galipremsagar
Copy link
Contributor Author

Talked with @galipremsagar and I will approve to unblock things, but Prem will address some of my feedback in a follow up PR.

Follow-up PR: #15485

rapids-bot bot pushed a commit that referenced this pull request Apr 10, 2024
This PR fixes an issue where `listJobsForWorkflowRun` returns only 30 jobs details by default and we need to paginate and load the rest all of the job details to be able to filter jobs.

This PR also address review comments in #15369

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)

URL: #15485
@mroeschke mroeschke mentioned this pull request May 21, 2024
3 tasks
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
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants