Skip to content

Add requirements.txt handling, utilize spinner class, and base image verification #110

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 10, 2021
Merged

Add requirements.txt handling, utilize spinner class, and base image verification #110

merged 1 commit into from
Sep 10, 2021

Conversation

KavinKrishnan
Copy link
Collaborator

@KavinKrishnan KavinKrishnan commented Sep 10, 2021 •
edited
Loading

Adding:

  • requirements.txt handling: users can provide a path to the requirements.txt file, overriding values provided by sdk
  • packager no longer uses it own spinner function, and utilizes the SDK spinner class in /util
  • added verification of user provided base images such that they are either nvcr.io/nvidia/cuda or nvcr.io/nvidia/pytorch images
  • additionally, added a --no-cache flag to CLI so user has the option to build docker images not using cache

@KavinKrishnan KavinKrishnan changed the title Add requirements.txt handling, utilize spinner class, and base image … Add requirements.txt handling, utilize spinner class, and base image verification Sep 10, 2021
gigony
gigony reviewed Sep 10, 2021
View reviewed changes
Copy link
Collaborator

@gigony gigony left a comment

Choose a reason for hiding this comment

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

Thanks Kavin for the update!

I left some comments here.

@gigony gigony added the architectural story Work related to architecture label Sep 10, 2021
@gigony gigony added this to the v0.1.0 milestone Sep 10, 2021
gigony
gigony approved these changes Sep 10, 2021
View reviewed changes
Copy link
Collaborator

@gigony gigony left a 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. Thank you Kavin!

Please rebase with signed commit to pass DCO check. Then let's merge this.

@KavinKrishnan KavinKrishnan force-pushed the kavink/packager_minor_fixes branch 5 times, most recently from c11660b to 12fd3a9 Compare September 10, 2021 23:43
@KavinKrishnan KavinKrishnan merged commit 74ba93c into main Sep 10, 2021
@KavinKrishnan
Add requirements.txt handling, utilize spinner class, and base image …
12fd3a9 
…verification

Modify packager to use SDK provided version number

Signed-off-by: kavink <[email protected]>
@gigony gigony mentioned this pull request Dec 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gigony gigony gigony approved these changes

Assignees

@KavinKrishnan KavinKrishnan

Labels
architectural story Work related to architecture
Projects
None yet
Milestone
v0.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@KavinKrishnan @gigony