Skip to content

client.images.push doesnt raise exception #3161

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

Open
obiii opened this issue Jul 20, 2023 · 5 comments · May be fixed by #3321
Open

client.images.push doesnt raise exception #3161

obiii opened this issue Jul 20, 2023 · 5 comments · May be fixed by #3321

Comments

@obiii
Copy link

obiii commented Jul 20, 2023

Hi,

We are using client.images.push to push docker images to Azure ACR. We are facing an internal authorization issue, but expected the API to raise an exception but it does not. It returns the generator str but raising an exception would be optimal here.

@odoublewen
Copy link

I came here to report exactly the same thing. This is kind of a big deal because it results in silent failures.

@Khushiyant
Copy link
Contributor

@milas What do you think?

@markokristian
Copy link

Also using this API and was surprised that our CI pipeline showed up as green even if the push failed.
Turns out this issue is the reason. When attaching a debugger it looked like the HTTP status code was 200. Using AWS ECR. The docker cli returned a non-zero exit code (2) for the same push call.

@ltanji01
Copy link

I was also experiencing the same thing with the following code. If I print the response, I will see the error but the try/catch statement won't detect it.

Image

@Khushiyant
Copy link
Contributor

Engine API doesn't raise error but return JSON with errorDetail and progressDetail [See here]

I am working on PR for this issue and will link soon so, at least a exception is raised in case of failure

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 a pull request may close this issue.

5 participants