close
Skip to content

Allow extension fields in the v3.4 version of the compose format - #452

Merged
dnephin merged 1 commit into
docker:masterfrom
dnephin:compose-allow-x-fields
Aug 25, 2017
Merged

Allow extension fields in the v3.4 version of the compose format#452
dnephin merged 1 commit into
docker:masterfrom
dnephin:compose-allow-x-fields

Conversation

@dnephin

@dnephin dnephin commented Aug 17, 2017

Copy link
Copy Markdown
Contributor

This has been a long requested feature (see docker/compose#2942 and many duplicated issues).

Any top-level key which starts with x- will be ignored by compose. This allows for users to:

  • include additional metadata in their compose files
  • create YAML anchor objects that can be re-used in other parts of the config

This matches a similar feature in the swagger spec definition: https://swagger.io/specification/#specificationExtensions

cc @shin- I'd like to make sure this is consistent with the next version of the v2.x format, so please comment if you think this is not appropriate

@shin-

shin- commented Aug 17, 2017

Copy link
Copy Markdown
Contributor

Is that preferable to a single, named top level section, like meta or similar? I feel like that would be less confusing / easier to explain. Also, aesthetically speaking x-something is very reminiscent of CSS browser hacks which everyone who's ever had to use them probably really wants to forget.

@dnephin

dnephin commented Aug 17, 2017

Copy link
Copy Markdown
Contributor Author

They are definitely inspired by css extensions. While they were annoying to use, I think they served their purpose in that it allowed browsers to experiment with features, and features that became popular were eventually incorporated into the official standard (I believe).

We could have a single pre-defined key instead of a pattern. meta to me reads more like it's actually part of "supported" keys, where as x-something I think should make it more obvious that it's ignored by docker and compose. HTTP (I think 1.0) had X-* headers for "extension" headers, but they eventually dropped that recommendation, so there is definitely precedence for the x- prefix from both swagger and http.

Maybe x-ignored or x-extra or something like that? Open to other suggestions as well. Using the pattern x- just makes the naming problem a little easier.

If the concern is documentation, we could document a single x-meta, and let savvy users discover that they can actually use anything.

@tbeadle

tbeadle commented Aug 19, 2017

Copy link
Copy Markdown

Can this also be added for 2.x config files?

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 🐯

Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin
dnephin force-pushed the compose-allow-x-fields branch from 569050b to 2a1857e Compare August 22, 2017 14:43
@codecov-io

codecov-io commented Aug 22, 2017

Copy link
Copy Markdown

Codecov Report

Merging #452 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
- Coverage   47.02%   47.01%   -0.01%     
==========================================
  Files         198      198              
  Lines       16336    16334       -2     
==========================================
- Hits         7682     7680       -2     
  Misses       8260     8260              
  Partials      394      394

@cpuguy83 cpuguy83 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.

It makes sense to have an easy way to have the yaml parser ignore some piece of data.
I'm not sure it's worth it to add a top-level key to namespace these objects.

Would definitely like to hear other maintianers feedback.

@vieux

vieux commented Aug 24, 2017

Copy link
Copy Markdown
Contributor

I like the way it's done now, so you can have extra keys close to the object they relate to.

@vieux vieux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dnephin
dnephin merged commit d83752c into docker:master Aug 25, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.08.0 milestone Aug 25, 2017
@dnephin
dnephin deleted the compose-allow-x-fields branch August 25, 2017 16:37
@dfee

dfee commented Aug 25, 2017

Copy link
Copy Markdown

Let the trumpets play!

@tbeadle

tbeadle commented Aug 25, 2017

Copy link
Copy Markdown

Any input on getting this added for 2.x version files? It shouldn't be a breaking change. My company is not able at this time to use 3.x files, but would really like to have this ability.

@shin-

shin- commented Aug 25, 2017

Copy link
Copy Markdown
Contributor

@tbeadle Yes, it's in our plans 👍

@casperdcl

Copy link
Copy Markdown

winning

@schmunk42

Copy link
Copy Markdown

Not very happy with the x- hack beauty but I can live with that.

As said here: moby/moby#31101 (comment) - I'd also have preferred another syntax, ie. .something, but I can also live with it. I just thought to speak this out, since it's not in a stable release yet.

@rdxmb

rdxmb commented Sep 21, 2017

Copy link
Copy Markdown
Contributor

.something indeed would be great.

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.