-
Notifications
You must be signed in to change notification settings - Fork 19
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
docs(ksql): Add ksqlDB guide #26
base: main
Are you sure you want to change the base?
Conversation
configuration/helm-charts/ksqldb.md
Outdated
|
||
## Configuration | ||
|
||
ksqlDB configuration is specified in `yamlApplicationConfig` section of `values.yaml` file. |
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.
Hi, thanks for the PR. This part only applies to users running the app via helm charts, can we add some short info for other types of installations as well? Some might not understand how to apply this to their configs.
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.
@Haarolean Explicitly added that containers based on docker and docker-compose can also be configured using environment variables. (48db4fa)
configuration/helm-charts/ksqldb.md
Outdated
properties: | ||
security: | ||
protocol: SSL | ||
bootstrapServers: b-2.<REDACTED>.<REDACTED>.c3.kafka.ap-northeast-2.amazonaws.com:9094,b-1.<REDACTED>.<REDACTED>.c3.kafka.ap-northeast-2.amazonaws.com:9094 |
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 believe we can remove (omit) unnecessary properties, unrelated to ksql
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.
@Haarolean I removed unrelative values at 9cbdaf9
configuration/helm-charts/ksqldb.md
Outdated
@@ -0,0 +1,60 @@ | |||
--- | |||
description: Integrating ksqlDB with Kafbat UI |
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 believe this can be moved out of helm-charts folder as this brings the overall knowledge about setting up ksql
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.
Moved under the more relevant directory configuration
(5e1ea8b)
Omitted unrelative values to increase readability * https://github.com/kafbat/ui-docs/pull/26\#discussion_r1874539210 (feedback) Signed-off-by: younsl <[email protected]>
* Go to the more relevant directory 'configuration' Signed-off-by: younsl <[email protected]>
Request change: https://github.com/kafbat/ui-docs/pull/26\#discussion_r1874539160 Signed-off-by: younsl <[email protected]>
@Haarolean Hi. Thanks for the detailed review. I've made all the changes you asked for. |
Add a Helm chart setup guide for connecting ksqlDB to Kafbat UI, as the initial setup lacks a ksqlDB connection guide for operators.