Skip to content

[Compose] Cast values to expected type after interpolating values#601

Merged
dnephin merged 4 commits into
docker:masterfrom
dnephin:compose-cast-after-interpolate
Oct 26, 2017
Merged

[Compose] Cast values to expected type after interpolating values#601
dnephin merged 4 commits into
docker:masterfrom
dnephin:compose-cast-after-interpolate

Conversation

@dnephin
Copy link
Copy Markdown
Contributor

@dnephin dnephin commented Oct 4, 2017

Fixes #229

Required a change to the order of operations to handle interpolation before schema validation. This should be safe because interpolation should not require any specific structure.

cc @shin-
@vdemeester sorry in advance for conflicts with #569 if this merges first

Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin dnephin requested a review from vdemeester as a code owner October 4, 2017 21:40
@dnephin dnephin force-pushed the compose-cast-after-interpolate branch 2 times, most recently from 3dd96e1 to ea107b5 Compare October 4, 2017 21:50
out := make([]interface{}, len(value))
for i, elem := range value {
interpolatedElem, err := recursiveInterpolate(elem, mapping)
interpolatedElem, err := recursiveInterpolate(elem, path, opts)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think maybe lists should have a [] item in the path to differentiate a path to a scalar or a path to an item in the list.

@dnephin dnephin requested review from thaJeztah and vieux October 5, 2017 15:08
@dnephin dnephin force-pushed the compose-cast-after-interpolate branch from ea107b5 to eff766d Compare October 5, 2017 15:37
Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin dnephin force-pushed the compose-cast-after-interpolate branch from eff766d to af8f563 Compare October 5, 2017 16:03
@seriousben
Copy link
Copy Markdown

Would be amazing to get this soon :)

Copy link
Copy Markdown
Contributor

@vieux vieux left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@vdemeester vdemeester left a comment

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 b68c3d0 into docker:master Oct 26, 2017
@dnephin dnephin deleted the compose-cast-after-interpolate branch October 26, 2017 16:20
@GordonTheTurtle GordonTheTurtle added this to the 17.11.0 milestone Oct 27, 2017
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.

5 participants