Skip to content
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

Add composefile schema 3.7 with platform #1081

Closed
wants to merge 1 commit into from

Conversation

vdemeester
Copy link
Collaborator

@vdemeester vdemeester commented May 24, 2018

It's ignored for now (docker stack deploy) and will print a warning if present.

Fixes #1050

That being said, I think one easy fix to that issue would be to add platform to v3 schema(s) — but it would start at schema 3.7and printing a warning when issuing a docker stack deploy that this field is not supported (same way we do it for build). It would allow to more easily updated from v2 to v3 for users and help us not make v2 and v3 schema diverge that much — at least not having elements in v2 that are not in v3…

Also linked to docker/compose#5985

Signed-off-by: Vincent Demeester [email protected]

@vdemeester vdemeester force-pushed the compose-v3-platform branch 2 times, most recently from 17e5af6 to e466323 Compare May 24, 2018 12:40
@silvin-lubecki silvin-lubecki changed the title Add composefile schema 3.7 with platfrom Add composefile schema 3.7 with platform May 24, 2018
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shin-
Copy link
Contributor

shin- commented May 24, 2018

Are we really okay with adding non-Swarm compatible options to v3? I'm asking because it's a significant depart from our strategy so far (with the exception of build sub-options) and I'm curious how it'll affect user experience.

I'm guessing the rationale is that Swarm (/kube?) will support the option eventually?

@vdemeester
Copy link
Collaborator Author

I'm guessing the rationale is that Swarm (/kube?) will support the option eventually?

Yes, that's part of the rationale — other part is I really wish we have as divergence between v2 and v3 composefile evolution 👼

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this requires more eyes; LCOW (and thus the --platform option) is still experimental, and things may change (possibly including the format of the --platform string (e.g., on Windows, some situations not only require the platform and architecture, but also OS version 😞 (IIRC))

I'm also a bit out of sync with the discussion around that (i.e, what format the --platform should take; string, or structured approach)

ping @johnstep @jhowardmsft PTAL

@jetersen
Copy link

How rigid do you want to be? Docker compose should perhaps support an experimental flag just like the docker for windows? So that we can get to try out the new stuff. Maybe I am asking for too much, I don't know. 🤔

@thaJeztah
Copy link
Member

How rigid do you want to be?

Well, a spec, once published, cannot be changed, so that's why I want to be sure it's finalised and agreed on.

Docker compose should perhaps support an experimental flag

Having this as an extension field (x-platform: foo) would be an option for that

@vdemeester
Copy link
Collaborator Author

So the x-* should be handled by the following PR : #1097

That said, platform is already present in v2.. and if that version of v2 has been released, we can't really transform it to a x-platform and thus, I really think we should at least have it in v3 even though it will be ignored. Otherwise (i.e. v2.4 support is not shipped in a release), we could transform it to `x-platform)

cc @dnephin @shin-

It's ignored for now (docker stack deploy) and will print a warning if present.

Signed-off-by: Vincent Demeester <[email protected]>
@vdemeester
Copy link
Collaborator Author

Closing this as it staled 😓

@vdemeester vdemeester closed this Feb 28, 2019
@jetersen
Copy link

no worries @vdemeester fixed by the auto-detection, so it's fine :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

platform support is missing in version 3.5 and 3.6
6 participants