Skip to content
This repository has been archived by the owner on Mar 18, 2024. It is now read-only.

Fixes #56 - Returning an error to the client if the image was not succes... #57

Closed
wants to merge 1 commit into from

Conversation

diptanu
Copy link

@diptanu diptanu commented Dec 10, 2014

Hi,

I am sending a PR for the #56

I am checking for the presence of the word errorDetail in the response. Please let me know if there is a more efficient solution than this.

@samalba
Copy link
Owner

samalba commented Dec 11, 2014

LGTM

@diptanu
Copy link
Author

diptanu commented Dec 11, 2014

@samalba Thanks! Can this be merged now?

@samalba
Copy link
Owner

samalba commented Dec 11, 2014

@aluzzardi @vieux Could you have a quick look?

@@ -122,4 +135,5 @@ func TestContainerLogs(t *testing.T) {
t.Fatalf("expected stderr log line \"%s\" to end with \"%s\"", line, expectedSuffix)
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: Why the extra empty line?

Copy link
Author

Choose a reason for hiding this comment

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

Will remove it.

@donhcd
Copy link
Collaborator

donhcd commented Dec 12, 2014

my pr #49 has the same goal. considering you've added tests and stuff, @diptanu maybe you could borrow my json unmarshalling code and add it into your pr? It looks at the error and returns that as the error. It could also return the errorDetail - I just wasn't sure which was the right one. I believe that if the image is not found, the two are the same.

@diptanu
Copy link
Author

diptanu commented Dec 15, 2014

@donhcd Sorry for the delayed response on this, as I am out on a vacation! Yeah, just looked at your PR and I have something similar now in terms of fetching the appropriate error message.

One questions for @vieux and @samalba - Can I assume that every json literal would be delimited by \n and the last line of the response from the docker daemon would contain the appropriate error message if the image download fails for some reason?

Would love to make sure my assumptions are correct before I submit the updated PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants