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

fix(helm): add inheritedParameterContexts option to NiFi ParameterCo… #475

Conversation

Roger1uphealth
Copy link
Contributor

@Roger1uphealth Roger1uphealth commented Nov 3, 2024

Q A
Bug fix? yes
New feature? no
API breaks? no
Deprecations? no
Related tickets partially #165
License Apache 2.0

What's in this PR?

This change will add the option to include inheritedParameterContexts in nifi-parameter-context helm template

Why?

Inherited parameter contexts were not being deployed as expected due to the missing field in the Helm template.

Checklist

  • Implementation tested
  • Error handling code meets the guideline
  • Logging code meets the guideline
  • User guide and development docs updated (if needed)
  • Append changelog with changes

To Do

  • If the PR is not complete but you want to discuss the approach, list what remains to be done here

@Roger1uphealth Roger1uphealth changed the title fix(helm): add inheritedParameterContexts support to NiFi ParameterCo… fix(helm): add inheritedParameterContexts option to NiFi ParameterCo… Nov 3, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
@juldrixx
Copy link
Contributor

juldrixx commented Nov 3, 2024

And can you regenerate the Helm doc using helm docs?

cd helm/nifi-cluster
helm-docs

Co-authored-by: Juldrixx <[email protected]>
Copy link
Contributor

@juldrixx juldrixx left a comment

Choose a reason for hiding this comment

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

Can you move it to ### Added instead of ### Fixed Bugs.

And put a capital letter on Chart in [Helm chart].

Copy link
Contributor

@juldrixx juldrixx left a comment

Choose a reason for hiding this comment

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

Can you add it in the Unreleased section at the beginning of the file?

Copy link
Contributor

@juldrixx juldrixx left a comment

Choose a reason for hiding this comment

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

Can you add the field in values.yaml too with a comment that will be part of the generated documentation like it's done for secretRefs and parameters.

@Roger1uphealth
Copy link
Contributor Author

@juldrixx thanks for quick review and patience. I think it's in a spot where it can be reviewed again when you have time.

@juldrixx
Copy link
Contributor

juldrixx commented Nov 4, 2024

Thanks for your contribution🙌.

@juldrixx juldrixx merged commit 9aba96e into konpyutaika:master Nov 4, 2024
5 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.

2 participants