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

[EN-158] allow override compose.yaml to REMOVE elements #380

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

ndeloof
Copy link
Collaborator

@ndeloof ndeloof commented Mar 30, 2023

this is an alternative to #379

using a custom yaml tag !reset (final name to be debated), this let user define compose.override.yaml to REMOVE elements from original model.
Also supports removal from maps, like environment, but miss support for removal from list (but feasible)

services:
  foo:
    image: foo
    build: !reset  
    environment:
      FOO: !reset

Copy link
Member

@milas milas left a comment

Choose a reason for hiding this comment

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

❤️

The !reset name is good with me. Other alternative I can think of would be !default. (I also like !unset but think that might be slightly inaccurate)

loader/null.go Outdated
return p._applyNullOverrides(reflect.ValueOf(target), tree.NewPath())
}

func (p *ResetProcessor) _applyNullOverrides(val reflect.Value, path tree.Path) error {
Copy link
Member

Choose a reason for hiding this comment

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

Nit - can just be applyNullOverrides 😛

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof requested a review from laurazard March 30, 2023 20:02
@ndeloof ndeloof changed the title allow override compose.yaml to REMOVE elements [EN-158] allow override compose.yaml to REMOVE elements Apr 4, 2023
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

A clean way to erase values during composition process 👍
LGTM

@ndeloof
Copy link
Collaborator Author

ndeloof commented Apr 6, 2023

Once compose-spec/compose-spec#330 is approve I'll propose changes there to introduce support for this mechanism

@ndeloof ndeloof merged commit 2c30658 into compose-spec:master Apr 25, 2023
8 checks passed
zackgalbreath added a commit to Kitware/CDash that referenced this pull request Jun 9, 2023
For now, this seems to be the best we can do.

Once docker compose v2.19 is released we should be able to
update the worker service by "extending" the cdash service,
removing the need for manually editing the production config YAML.

In particular, we need the new "!reset" feature to prevent both the web
service and the worker from attempting to bind port 443.
compose-spec/compose-go#380
zackgalbreath added a commit to Kitware/CDash that referenced this pull request Jun 9, 2023
For now, this seems to be the best we can do.

Once docker compose v2.19 is released we should be able to
update the worker service by "extending" the cdash service,
removing the need for manually editing the production config YAML.

In particular, we need the new "!reset" feature to prevent both the web
service and the worker from attempting to bind port 443.
compose-spec/compose-go#380
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants