-
Notifications
You must be signed in to change notification settings - Fork 126
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
Add operate value docs #143
Conversation
Rename serviceAccount.created to serviceAccount.enabled to align with other variable names
To reduce duplication and avoid drifting away (values) values are specified and documented in the parent values.yaml file. This will overwrite anyway values from the subchart. This creates one single place of truth we only need to manage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
I assume there will be a follow up at some point to document everything in the README?
# Logging configuration for the operate logging. This template will be directly included in the operate configuration yaml file | ||
logging: | ||
level: | ||
ROOT: INFO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Won't this be a bit confusing? I understand whatever under the logging config is going to be added to Operate's configuration file, but right now these look like variables which don't follow the same convention. Additionally, I think they can still be used as variable mistakenly? Would it make more sense to turn this into an actual string, like we do for Zeebe's log4j2.xml and the likes? Or would that be too error prone since the YAML isn't validated then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally agree. I was also unsure about these properties. It seems also that they are out dated. I asked for feedback from the operate team. https://camunda.slack.com/archives/C9B5270DA/p1643803696900629
In the deployment guide it seems to reference only the log4j file, so we could do that. https://docs.camunda.io/docs/self-managed/operate-deployment/configuration/#logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do that separately then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up issue #148
Exactly this was my plan :) Thanks for the review! |
related to #124