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

#1257 Kapitan does not prune Kadet input #1258

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

JordanLloydHall
Copy link
Contributor

Fixes #1257

Proposed Changes

  • KapitanInputTypeKadetConfig.prune defaults to False instead of True (inherited).
  • Type hint for KapitanInputTypeJsonnetConfig.prune is consistent with KapitanInputTypeBaseConfig.prune.

@ademariag ademariag enabled auto-merge (squash) October 31, 2024 09:03
@@ -39,7 +39,7 @@ class KapitanInputTypeBaseConfig(BaseModel):
class KapitanInputTypeJsonnetConfig(KapitanInputTypeBaseConfig):
input_type: Literal[InputTypes.JSONNET] = InputTypes.JSONNET
input_paths: List[str]
prune: bool = False
prune: Optional[bool] = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @alledm.

What do you think of making this default to False in the baseconfig on line 36?

I also noticed that configuring this in the inventory (prune: false) config does not work

parameters:
  kapitan:
    compile:

    - output_path: manifests
      input_type: kadet
      output_type: yaml
      prune: false

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes makes sense. this is something that didn't notice. will fix it soon

Copy link
Contributor Author

@JordanLloydHall JordanLloydHall Oct 31, 2024

Choose a reason for hiding this comment

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

Ideally this should be default None as the setting precedence seems to be:

  1. This class' prune attribute.
  2. The prune cmd/dot setting.
  3. Default False.

Although while this key exists in the class at all, neither the cmd nor the dot files will do anything. This is because we check if the prune key exists rather than checking if an attempt to get it's value returns None at the relevant call sites.

@ademariag ademariag merged commit c84102a into kapicorp:master Oct 31, 2024
7 of 8 checks passed
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.

[bug]: KapitanInputTypeKadetConfig inherits prune from KapitanInputTypeBaseConfig
3 participants