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

Fix error and success handling on image pull #642

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

marusak
Copy link
Member

@marusak marusak commented Jan 4, 2021

We have shown error on every pull because we tried to parse the whole
response as one JSON, but in fact it is a group of JSONs. That meant we
failed to notice the fact that we were not showing any useful error when
downloading actually failed. (We always just shown that we could not
parse the response)

Fixes #640

@marusak marusak mentioned this pull request Jan 4, 2021
We have shown error on every pull because we tried to parse the whole
response as one JSON, but in fact it is a group of JSONs. That meant we
failed to notice the fact that we were not showing any useful error when
downloading actually failed. (We always just shown that we could not
parse the response)

Fixes cockpit-project#640
@marusak marusak changed the title Don't unnecessary fail to parse reply when pulling images Fix error and success handling on image pull Jan 4, 2021
@marusak
Copy link
Member Author

marusak commented Jan 4, 2021

I suspect that we will need naughty for containers/podman#8870 but lets see first what our bots think.

@marusak marusak requested a review from KKoukiou January 4, 2021 12:56
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

Hmm the API is weird here or is it me? libpod pull should fail properly when the pull fails and we should catch it in catch without searching for error in the successful response.

Your changes make the error show up, so we should merge them, but should we report a bug in podman for the API?

In addition, when in non-existent tag is used when downaloading an image, the API fail with a not-so-good error message.

:note the screenshot is with your change included.
Screen Shot 2021-01-04 at 14 05 38

@marusak
Copy link
Member Author

marusak commented Jan 4, 2021

Hmm the API is weird here or is it me? libpod pull should fail properly when the pull fails and we should catch it in catch without searching for error in the successful response.

Yeah, kinda what I expected as well. But it is streaming so it does not have chance to response with different error code.

In addition, when in non-existent tag is used when downaloading an image, the API fail with a not-so-good error message.

this is bug in podman, reported as containers/podman#8870

@marusak marusak merged commit d4cdfa7 into cockpit-project:master Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants