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

Relax kadet output type requirement. #710

Merged
merged 2 commits into from
Mar 17, 2021
Merged

Relax kadet output type requirement. #710

merged 2 commits into from
Mar 17, 2021

Conversation

Jean-Daniel
Copy link
Contributor

Proposed Changes

There is 2 issues with requiring BaseObj in kadet.

BaseObj as return type for main()

kadet Input expects just a mapping of filename -> dict. Requiring a BaseObj is confusing as it implies kadet expects a single object and not a list of files. This change adds support for raw dict as return type. It simplifies implementation of kadet modules as it remove the need to build a BaseObj just to returns a mapping of filenames to BaseObj.

For instance, the nginx example may be rewrite as:

def main():
    return {
      "nginx_deployment": kubelib.Deployment(name=name, labels=labels, containers=[nginx_container]),
      "nginx_service": kubelib.Service(name=name, labels=labels, ports=[svc_port], selector=svc_selector)
    }

instead of

def main():
    output = kadet.BaseObj()
    output.root.nginx_deployment = kubelib.Deployment(name=name, labels=labels, containers=[nginx_container])
    output.root.nginx_service = kubelib.Service(name=name, labels=labels, ports=[svc_port], selector=svc_selector)
    return output

BaseObj as a required type for generated objects.

While using BaseObj may be convenient to build a complex nested dict, this is not the single way to generate such structure, and imposing it prevents exploring other frameworks. This change also relax this restriction and support simple dict in addition of BaseObj so any generator/kadet module would be able to generates dict as they see fit.

Jean-Daniel Dupas and others added 2 commits March 13, 2021 09:29
There is 2 issues with requiring `BaseObj` in kadet.

  * BaseObj as return type for main(): kadet Input expects just a mapping of filename -> `dict`. Requiring a BaseObj is confusing as it implies kadet expect a single object and not a list of files. This change adds support for raw `dict` as return type. It simplify implementation of kadet modules as it remove the need to build a `BaseObj` just to return a mapping of filename.

  * BaseObj as a required type for generated objects. While using BaseObj may be convenient to build a complex nested dict, this is not the single way to generate such structure, and imposing it prevents exploring other frameworks. This change also relax this restriction and support simple `dict` in addition of `BaseObj` so any generator/kadet module would be able to generates dict as they see fit.
Copy link
Member

@ramaro ramaro left a comment

Choose a reason for hiding this comment

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

Very good point and thanks again @Jean-Daniel

@ramaro ramaro merged commit 826f618 into kapicorp:master Mar 17, 2021
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.

2 participants