Fix double-marshalling of cachefrom#1086
Conversation
| webTarget = webTarget.queryParam("cachefrom", c); | ||
| } | ||
| if (command.getCacheFrom() != null && !command.getCacheFrom().isEmpty()) { | ||
| webTarget = webTarget.queryParam("cachefrom", "[\"" + Joiner.on("\",\"").join(command.getCacheFrom()) + "\"]"); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Please reuse existing helper methods, + parameter should be escaped
There was a problem hiding this comment.
@KostyaSha good point! It's not a Map, but I created a similar utility class
Codecov Report
@@ Coverage Diff @@
## master #1086 +/- ##
==========================================
+ Coverage 58.1% 58.11% +0.01%
==========================================
Files 445 445
Lines 8819 8814 -5
Branches 537 536 -1
==========================================
- Hits 5124 5122 -2
+ Misses 3404 3401 -3
Partials 291 291
Continue to review full report at Codecov.
|
|
@KostyaSha I addressed the comments |
|
|
||
| public static String jsonEncode(Collection<String> imageIds) { | ||
| try { | ||
| return OBJECT_MAPPER.writeValueAsString(imageIds); |
There was a problem hiding this comment.
You added new encoder, didn't existing fit that i referenced?
There was a problem hiding this comment.
The existing one deals with maps
3 participants
The API for setting the --cache-from argument is:
which intuitively indicates that the set of strings passed in should be the imageIds to use for the cache. However this is not the case, and instead users have to "wrap" the arguments in an additional '[]', as illustrated by an existing test
BuildImageCmdIT.cacheFrom:As stated in the docker API https://docs.docker.com/engine/api/v1.37/#operation/ImageBuild
the
cachefromparameter is ahowever the current implementation takes the set of strings, where each string itself is an array, and creates a "cachefrom" parameter for each. In other words it seems to require another layer of marshalling.
I think the original intention for this API was to take the set of strings where each string is an image id and serialize these into a JSON array automatically. This PR does this.
This change is