Repository navigation
Add volume subpath option - #594
Conversation
| volume.ReadOnly = false | ||
| case "nocopy": | ||
| volume.Volume = &types.ServiceVolumeVolume{NoCopy: true} | ||
| case "subpath": |
There was a problem hiding this comment.
that sounds a bit weird to me: if we can detect use of subpath here, why not support this syntax ?
There was a problem hiding this comment.
The reason is to make it consistent with the Docker CLI where we don't support the short syntax, only the long --mount.
We decided to not support it in the short syntax is to avoid growing parsing complexity.
But if compose is fine with that, I'm happy to implement it.
There was a problem hiding this comment.
right, then maybe better have a generic default case rejecting any unsupported attribute
There was a problem hiding this comment.
The default branch seems to explicitly ignore the unsupported attributes:
Lines 98 to 102 in a1f149a
Also, in this case the option is supported, just not in the short syntax. Returning an "not supported" message could make the user think that it's not supported at all.
There was a problem hiding this comment.
how does the CLI report attempt to use such an attribute ?
There was a problem hiding this comment.
CLI errors out when an unknown attribute is used in both long and short syntax.
As for the subpath option specifically, it's not handled differently and will error out just the same as an invalid attribute.
$ docker run -v asddd:/v:volume-subpath=sub --rm -it alpine
docker: Error response from daemon: invalid mode: volume-subpath=sub.
There was a problem hiding this comment.
right, then I don't think compose should try to be more clever/friendly here OR subpath should be made an explicit short-syntax feature
There was a problem hiding this comment.
Fair enough. Removed the special case for the short syntax.
Only supported in the long syntax. Signed-off-by: Paweł Gronowski <[email protected]>
subpathproperty compose-spec#471Only supported in the long syntax.