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

awaitImageId() using BuildImageResultCallback issue #2184

Closed
AlexanderJell opened this issue Aug 17, 2023 · 1 comment · Fixed by #2305
Closed

awaitImageId() using BuildImageResultCallback issue #2184

AlexanderJell opened this issue Aug 17, 2023 · 1 comment · Fixed by #2305

Comments

@AlexanderJell
Copy link

We used the api to build a model that installs pip dependencies for a python application.

Running the pip install outputs "Sucessfully built <package" for packages that are built.
This caused the api to return the name due to the implementation of parsing the imageId in the BuildImageResultCallback class.

We fixed it by adding a lastStepDone boolean that waits for Step n/n done and then enables the imageId parsing by waiting for "Sucessfully built <...>"

Should I open a pr with our solution I think there is no disadvantage to wait for docker image build completion before parsing the image id?

@niloc132
Copy link
Contributor

niloc132 commented Feb 2, 2024

Just ran into this via https://github.com/bmuschko/gradle-docker-plugin, also using pip. My proposed solution was going to be to tweak onNext to null out this.imageId when an error is received. It could make sense to not parse until the stream was finished, but since BuildImageResultCallback.getImageId is private and can't be called until the POST /build is completed, it might be just as simple to check for a error before checking for the imageId.

private String getImageId() {
if (imageId != null) {
return imageId;
}
if (error == null) {
throw new DockerClientException("Could not build image");
}
throw new DockerClientException("Could not build image: " + error);
}

could instead be

 private String getImageId() { 
     if (error != null) { 
         throw new DockerClientException("Could not build image: " + error); 
     } 
  
     if (imageId != null) { 
         return imageId; 
     } 
  
    throw new DockerClientException("Could not build image"); 
 } 

niloc132 added a commit to niloc132/docker-java that referenced this issue Feb 2, 2024
This patch ensures that if the docker build commands result in logging
the string "Successfully built", the callback from that build will still
fail if there was an error in building.

Fixes docker-java#2184
eddumelendez pushed a commit that referenced this issue Feb 26, 2024
…2305)

This patch ensures that if the docker build commands result in logging
the string "Successfully built", the callback from that build will still
fail if there was an error in building.

Fixes #2184
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.

2 participants