Add support for declaring named volumes in compose files #2421
Conversation
| config = merge_services(config_file.config, next_file.config) | ||
| config_file = config_file._replace(config=config) | ||
|
|
||
| return build_services(config_file) | ||
|
|
||
|
|
||
| def process_config_file(config_file, service_name=None): | ||
| def process_config_file(config_file, service_name=None, version=None): |
There was a problem hiding this comment.
I think ServiceExtendsResolver.validate_and_construct_extends() will need to call this with a version as well.
Since version is a required parameter can we make it a position arg instead of a kwarg?
shin-
force-pushed
the
2110-compose_yml_v2
branch
3 times, most recently
from
b8ec0d4
to
8552ce1
Compare
shin-
force-pushed
the
2110-compose_yml_v2
branch
2 times, most recently
from
9a23f84
to
b1a551d
Compare
|
I think I addressed most of the points brought up in the earlier review, except for the documentation part. |
| next_file_version = get_version(next_file) | ||
| if version is None: | ||
| version = next_file_version | ||
| continue |
There was a problem hiding this comment.
If I'm reading this right, you could have a legacy main file and a version-1 override file, and the ultimate version would be 1.
It scares me that the version decided upon is dependent on the override files. I think it'd be better if we enforced that all files must be the same version.
There was a problem hiding this comment.
OK, I think I might be misreading this. The possible versions are actually 1 and 2, so what does it mean when we return None from get_version?
There was a problem hiding this comment.
When one config file is empty, as tested here. In that case we can't assume the version to be a specific one, but we don't want to fail either.
There was a problem hiding this comment.
Can't we just assume the version is 1? An "empty" v2 file should look like this, in my opinion:
version: 2There was a problem hiding this comment.
If you think that's better, that's an easy change to make.
| @@ -18,7 +18,7 @@ get_versions="docker run --rm | |||
| if [ "$DOCKER_VERSIONS" == "" ]; then | |||
| DOCKER_VERSIONS="$($get_versions default)" | |||
| elif [ "$DOCKER_VERSIONS" == "all" ]; then | |||
| DOCKER_VERSIONS="$($get_versions recent -n 2)" | |||
| DOCKER_VERSIONS="$($get_versions recent -n 1)" | |||
There was a problem hiding this comment.
Mind adding a TODO here to set this back to 2 after the first RC is released?
|
LGTM, I think we can address the remaining (documentation, and using existing volumes) after this is merged |
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Fixes a regression after the API changed to use Mounts. Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
* Bump default API version to 1.21 (required for named volume management) * Introduce new, versioned compose file format while maintaining support for current (legacy) format * Test updates to reflect changes made to the internal API Signed-off-by: Joffrey F <joffrey@docker.com>
Add volume configuration reference section. Signed-off-by: Joffrey F <joffrey@docker.com>
Also includes several bugfixes for resolution and validation. Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
When created through the compose file, volumes are prefixed with the name of the project they belong to + underscore, similarly to how containers are currently handled. Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
Assume version=1 if file is empty in get_config_version Empty files are invalid anyway, so this simplifies the algorithm somewhat. docker#2421 (comment) Don't leak version considerations in interpolation/service validation Signed-off-by: Joffrey F <joffrey@docker.com>
Signed-off-by: Joffrey F <joffrey@docker.com>
shin-
force-pushed
the
2110-compose_yml_v2
branch
from
d5bc144
to
1dcdd98
Compare
|
Squashed some commits + rebased. |
Add support for declaring named volumes in compose files
for current (legacy) format
Fixes #2110, replaces #2113
WIP: