-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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:
|
| "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 <[email protected]>
Signed-off-by: Laurent Goderre <[email protected]>
8cfcd98 to
3758a6b
Compare
- What I did
Add image mount subpath option to compose schema
- Human readable description for the release notes