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.

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

Fix pulling from nvcr.io #285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix pulling from nvcr.io #285

wants to merge 1 commit into from

Conversation

uvNikita
Copy link
Contributor

@uvNikita uvNikita commented Sep 8, 2020

Based on #238 with my suggestions applied.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 78.351% when pulling 50e135d on uvNikita:nvcr into 771dcc4 on NERSC:master.

@scanon
Copy link
Member

scanon commented Sep 8, 2020

Nikita,

The latest updates added an option to use some external tools from the broader OCI community to handle pulling and unpacking images. This may be a more robust way to deal with these other repositories. But I'll take a look at the PR.

scanon
scanon reviewed Sep 8, 2020
View reviewed changes
imagegw/shifter_imagegw/dockerv2.py
# The json content type can include a extra bit defining the character set
# so we just check the beginning. Worst case is the json.loads will throw
# an error just after this.
if not resp.getheader('content-type').startswith('application/json'):
Copy link
Member

Choose a reason for hiding this comment

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

Why was this dropped? Does the nvcr case not return json?

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 part is from your original PR, but I can check if it's still needed.

scanon
scanon reviewed Sep 8, 2020
View reviewed changes
Copy link
Member

@scanon scanon left a comment

Choose a reason for hiding this comment

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

This looks good as far as I can tell. I would like to run it and make sure there aren't any edge cases that fail.

@uvNikita
Copy link
Contributor Author

uvNikita commented Sep 9, 2020

Nikita,

The latest updates added an option to use some external tools from the broader OCI community to handle pulling and unpacking images. This may be a more robust way to deal with these other repositories. But I'll take a look at the PR.

Sounds good! Is there something I should do to enable this new pulling method?

@scanon
Copy link
Member

scanon commented Sep 9, 2020

Here is the documentation on the external mode option....

https://github.com/NERSC/shifter/blob/master/doc/skopeo.rst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scanon scanon scanon left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@uvNikita @coveralls @scanon