-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add interpolation type cast for max_replicas_per_node #2182
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 interpolation type cast for max_replicas_per_node #2182
Conversation
|
Please sign your commits following these rules: $ git clone -b "fix-max_replicas_per_node_interpolation" git@github.com:rumpl/cli.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
3041997 to
d090152
Compare
silvin-lubecki
left a comment
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.
LGTM
thaJeztah
left a comment
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.
overall LGTM, but left a comment
cli/compose/loader/loader_test.go
Outdated
| func TestLoadWithInterpolationCastFull(t *testing.T) { | ||
| dict, err := ParseYAML([]byte(` | ||
| version: "3.7" | ||
| version: "3.9" |
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.
Could you change this to version 3.8? This feature was added in 19.03 (through #1612), which has compose schema 3.8 as maximum
Changing to version 3.8 would allow us to back port this fix
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
Fixes docker-archive-public/docker.app#688 Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
d090152 to
cb29ef6
Compare
thaJeztah
left a comment
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.
LGTM, thanks!
Fixes docker-archive-public/docker.app#688
- What I did
Added interpolation type cast mapping for missing
deploy.placement.max_replicas_per_node.- How to verify it
Changed the unit test
- A picture of a cute animal (not mandatory but encouraged)
