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

Update Kafka install docs #4677

Merged
merged 11 commits into from
Feb 24, 2023
Merged

Update Kafka install docs #4677

merged 11 commits into from
Feb 24, 2023

Conversation

sakoush
Copy link
Member

@sakoush sakoush commented Feb 21, 2023

What this PR does / why we need it:

Update Strimzi Kafka installation README.md and link to it from docs.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Copy link
Contributor

@RafalSkolasinski RafalSkolasinski left a comment

Choose a reason for hiding this comment

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

nice one! some minor suggestions

kafka/strimzi/README.md Outdated Show resolved Hide resolved
kafka/strimzi/README.md Show resolved Hide resolved
kafka/strimzi/README.md Outdated Show resolved Hide resolved
```

Create our Kafka cluster
Create Kafka cluster in `seldon-mesh` namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

I would highlight here two options:

  • (1) using our provided helm chart
  • (2) providing minimal kafka.yaml that can be applied directly

we could use "tabs" to nicely show the alternatives

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not entirely convinced that we should add here a minimal kafka.yaml as we are describing how to install the Helm chart that we are providing for kafka. Perhaps in some other place we could add this minimal kafka setup but I am not sure if it adds much given that we have the Chart and users can refer to the more details strimzi setup that I linked in the docs, IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to sync with @agrski about what purpose he's seen this Helm chart to serve.
I see it as nice convenience around customizing Strimzi Kafka install for dev / testing setup.

I would probably lean towards directing users to use Strimzi directly if they do in-cluster Kafka with guidance from our side limited to what configuration options are relevant to use it with Core V2 pipelines.

I see value from providing Helm chart and adding some notes how it can be used for creating Kafka cluster but would not want us to be seen as authority on how to configure production Kafka clusters.

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe we can add a note at the top of this file to say it is for SCv2 dev purposes and refer to strimzi docs for details?

@sakoush sakoush merged commit 7696e0f into SeldonIO:v2 Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants