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

[FEA]: Avoid running stubgen in CI #766

Closed
2 tasks done
dagardner-nv opened this issue Mar 15, 2023 · 1 comment · Fixed by #1111
Closed
2 tasks done

[FEA]: Avoid running stubgen in CI #766

dagardner-nv opened this issue Mar 15, 2023 · 1 comment · Fixed by #1111
Labels
feature request New feature or request

Comments

@dagardner-nv
Copy link
Contributor

Is this a new feature, an improvement, or a change to existing functionality?

Change

How would you describe the priority of this feature request

High

Please provide a clear description of problem this feature solves

Running stubgen in CI requires the Nvidia drivers to be installed in the CI image. This unfortunately locks us to a specific driver version, and ops would like to accelerate adopting new versions.

Describe your ideal solution

Include the stubs as part of the source code and add a CI check to make sure they are up to date

  • This would shift generating the stubs to the developer (can still be part of the build. The dev just needs to commit them)
  • The test in CI could happen in the test stage which already has a GPU
  • The test would be as simple as running the stubgen code and seeing if there are any diffs. Similar to what we do for clang-format

Describe any alternatives you have considered

  1. See if the cuda-toolkit driver stubs work the same as cudatoolkit
    If we can load the stubs but not run anything, that would work for us. The current stubs throw an error on load.
    This is a long shot but requires the least amount of work

  2. Generate the stubs in a pre-commit hook

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I have searched the open feature requests and have found no duplicates for this feature request
@dagardner-nv dagardner-nv added the feature request New feature or request label Mar 15, 2023
@github-actions github-actions bot added the Needs Triage Need team to review and classify label Mar 15, 2023
@jarmak-nv jarmak-nv removed the Needs Triage Need team to review and classify label Jun 12, 2023
@mdemoret-nv
Copy link
Contributor

Steps:

  • Remove .pyi files from .gitignore
  • Always copy .pyi back to source tree even if inplace is off
  • Change test stage in CI to perform a full build
  • Add a step before tests to verify no git diffs (i.e. the stubs are up to date)
  • Break up CI scripts into smaller components so they can be timed independently (i.e. step for conda env, build, run tests)

@rapids-bot rapids-bot bot closed this as completed in #1111 Aug 4, 2023
rapids-bot bot pushed a commit that referenced this issue Aug 4, 2023
This PR changes the stub generation to be committed into the repo instead of as part of the build. This will allow CI steps that need the PYI files to not require a GPU as well.

This works by always copying the PYI files back to the source tree so they will show up as diffs. Any diffs during the test phase will be reported as an error.

Closes #766

Authors:
  - Michael Demoret (https://github.com/mdemoret-nv)
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Christopher Harris (https://github.com/cwharris)
  - David Gardner (https://github.com/dagardner-nv)

URL: #1111
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants