-
Notifications
You must be signed in to change notification settings - Fork 2k
Add image mount subpath option to compose schema #5943
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
base: master
Are you sure you want to change the base?
Conversation
4cc8e10
to
6dc1bbb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5943 +/- ##
==========================================
- Coverage 59.43% 59.43% -0.01%
==========================================
Files 358 358
Lines 29769 29765 -4
==========================================
- Hits 17694 17690 -4
- Misses 11107 11111 +4
+ Partials 968 964 -4 🚀 New features to boost your workflow:
|
@@ -316,6 +316,12 @@ | |||
"nocopy": {"type": "boolean"} | |||
} | |||
}, | |||
"image": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the 3.13 schema was already released, we probably need a 3.14 schema; the way we approached that in previous ones was to have a PR with 2 separate commits; the first commit would copy the schema as-is to the new version, then a second commit to apply the changes; this allows for reviewing the changes that are added in isolation; for example, see this one that did the last update;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should say "no changes" for that first commit; except for updating the version in the schema 😅 (but the linked PR should probably give that information)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! So, the first commit probably should update at least the defaultVersion
as well, and we may want to update some of the versions used elsewhere (see a731722)
For docker stack
, the default is nowadays to pick "latest" (defaultVersion
), but it allows specifying a specific version to downgrade features (for compatibility with different versions of the CLI).
Just to be sure; was this change in the schema also added in the compose-spec (which is what docker compose uses)? https://github.com/compose-spec/compose-spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not. I wasn't sure how the two were related
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's .. complicated..
The "v3" compose schema in this repository pre-dates the compose-spec, which was based on merging the "v2" and "v3" specs, but also removed versioning and re-introduced features that were deprecated in v3.
Ultimately, we should look at migrating this code to use the compose-spec, but there are some subtle differences that need future discussion before we do so.
In the meantime, for features that are to be supported by the v3 schema in this repository, we basically take the approach to;
- propose changes in the compose-spec
- then implement them in https://github.com/compose-spec/compose-go (for docker-compose)
- and implement them here (docekr/cli)
The above is to avoid the v3 schema to be introducing a feature that's not accepted by the compose-spec, it's ok(ish) to have features in the compose-spec that are not (yet) supported by the v3 schema, but we want to avoid the reverse (v3 adding features that are not supported in the compose-spec).
If needed, I'm sure @ndeloof or @glours would be able to help with the steps for the compose-spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I had in mind to stop adding more options to volumes
and introduce a mount
section, to better align with the docker CLI UX, as those are not strictly equivalent.
701447a
to
8cfcd98
Compare
Signed-off-by: Laurent Goderre <laurent.goderre@docker.com>
Signed-off-by: Laurent Goderre <laurent.goderre@docker.com>
8cfcd98
to
3758a6b
Compare
- What I did
Add image mount subpath option to compose schema
- Human readable description for the release notes