Implement override and reset analog to docker-compose #830
Conversation
SebTM
force-pushed
the
override_reset
branch
2 times, most recently
from
90c1a27
to
c47dddb
Compare
…ocker.com/compose/compose-file/13-merge/) Signed-off-by: Sebastian Sellmeier <mail@sebastian-sellmeier.de>
| return service | ||
|
|
||
| try: | ||
| tagObj.value = serivce |
There was a problem hiding this comment.
No you need to still normalize the values of the Override-tag to be valid in the end (as they are the ones used) but pass the override-tag through to the recursive merge to do the actual override. I used try to determinate if there is a tagObj defiend and if so get the normalized values to the tagObj and return it otherwise return the normalized service-definition.
There was a problem hiding this comment.
Also this isn't my most recent version anymore as I'm locally working an a version based on the latest https://github.com/maurerle/podman-compose but for now using docker-compose again.
There was a problem hiding this comment.
serivce seems to be non-existing variable though.
There was a problem hiding this comment.
Edit: You are all right - missed the typo but that's not an issue/the latest state and needs a rebase.
How likely are my chances to get this approach merged, if I resubmit it?
There was a problem hiding this comment.
Good question, haven't looked into viability of the PR so far. Could you point me to the part of the compose spec that this implements? I didn't find with couple minutes of googling and better I don't make an error here :) Whoops, it's in PR description
Overall I think podman-compose should support all things possible in non-swarm compose that can be implemented on podman. We may argue about the approach of implementation though.
What I will definitely ask that's not part of the PR right now is unit and integration tests.
| class OverrideTag(yaml.YAMLObject): | ||
| yaml_dumper = yaml.Dumper | ||
| yaml_loader = yaml.SafeLoader | ||
| yaml_tag = u'!override' |
There was a problem hiding this comment.
If I remember correctly, u prefix is from python 2 times. Am I correct that it's not needed here?
| for item in value: | ||
| values.append(item.value) | ||
|
|
||
| self.value = values |
There was a problem hiding this comment.
self.value = [item.value for item in value]
| @@ -1629,7 +1709,7 @@ def _parse_compose_file(self): | |||
| compose["services"] = resolved_services | |||
| if not getattr(args, "no_normalize", None): | |||
| compose = normalize_final(compose, self.dirname) | |||
| self.merged_yaml = yaml.safe_dump(compose) | |||
| self.merged_yaml = yaml.dump(compose) | |||
| return service | ||
|
|
||
| if isinstance(service, OverrideTag): | ||
| tagObj = service |
| if isinstance(service, OverrideTag): | ||
| tagObj = service | ||
| service = tagObj.value | ||
| print(subdir) |
|
Just to let you all know - I'm still interested in this feature but I have to put the PR from my side on-hold so if someone needs it / want's to take over - feel free 🙏🏻 |
https://docs.docker.com/compose/compose-file/13-merge/