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

Document how to use deployment along with Kafka #207

Open
desmarchris opened this issue Jun 9, 2021 · 14 comments
Open

Document how to use deployment along with Kafka #207

desmarchris opened this issue Jun 9, 2021 · 14 comments
Labels
type:docs 📖 Improvements or additions to documentation type:enhancement ✨ New feature or request

Comments

@desmarchris
Copy link
Contributor

desmarchris commented Jun 9, 2021

Currently, you must add a new section to the values file as well as edit a config map to enable Kafka. This is shown here: https://github.com/RasaHQ/rasa-x-helm/pull/163/files

Definition of Done
As a user I can view documentation on how to enable Kafka using helm values.

@b-quachtran
Copy link

@RasaHQ/infrastructure-squad Are there any plans for supporting this out of the box in the near future?

@indam23
Copy link
Contributor

indam23 commented Sep 6, 2021

You can add additional endpoints to rasa.additionalEndpoints, so it's not necessary to modify the config map. I just realized this after opening the PR above 😅

@tczekajlo
Copy link
Contributor

@RasaHQ/infrastructure-squad Are there any plans for supporting this out of the box in the near future?

Not really, I mean you can already do this with the rasa helm chart as @melindaloubser1 mentioned.

For the rasa-x-helm chart, we won't add it, the goal that we have is to separate rasa-x from rasa oss.

@indam23
Copy link
Contributor

indam23 commented Oct 4, 2021

I'd suggest we close this since it's already supported/possible at least.

@indam23 indam23 closed this as completed Oct 4, 2021
@desmarchris desmarchris reopened this Oct 4, 2021
@desmarchris
Copy link
Contributor Author

More of my customers use Kafka than Rabbit, so I think it should be a first class citizen. We shouldn't have to send them examples on how to enable Kafka

@indam23
Copy link
Contributor

indam23 commented Oct 4, 2021

That's a fair point, but RabbitMQ is a service started by the helm deployment itself, while Kafka is always external. I think (@tczekajlo correct me if I'm wrong) that is why rabbit got its own section, while Kafka endpoints have to be added in a subsection.

@desmarchris
Copy link
Contributor Author

desmarchris commented Oct 4, 2021

while Kafka is always external

are you sure? I didn't think this was the case

@indam23
Copy link
Contributor

indam23 commented Oct 4, 2021

well now I'm not sure anymore 😅 But I guess since it requires forking the helm charts to make kafka a service as part of the deployment, I'd never seen anyone do it that way.

@desmarchris
Copy link
Contributor Author

I still think there'd be value in adding a simple kafka section like we have in OSS endpoints instead of adding them to a generic area

@indam23
Copy link
Contributor

indam23 commented Oct 5, 2021

In that case we can re-open the closed PR since that's what it did - @tczekajlo since it's not really a functional change and it seems it would be more intuitive for users, is that fine?

@tczekajlo
Copy link
Contributor

Here are my thoughts 😄 :

  1. For now, a way to add extra configuration is disabling rabbitmq (rabbitmq.enabled=false) and add Kafka section to rasa.additionalEndpoints
#values.yaml
rasa:
  additionalEndpoints:
    event_broker:
      type: "kafka"
      # security protocol
      security_protocol: SASL_PLAINTEXT
      # kafka topic
      topic: my_topic
      # kafka host
      url: localhost
      # SASL credentials
      sasl_username: username
      sasl_password: password

rabbitmq:
  enabled: false
  1. @melindaloubser1 is right, rabbitmq got his own section because it's a component included in the helm chart.
  2. Following the helm good practices, adding kafka section will imply that a Kafka helm chart is included and used as a sub-chart, and you can configure it through the kafka section, which is not true.

@desmarchris As there is a way to configure Kafka via values without editing the helm chart itself, my proposition would be just to document how to enable Kafka, WDYT?

cc @RasaHQ/infrastructure-squad

@desmarchris
Copy link
Contributor Author

ok fine ;) can we add it to the documentation?

@RASADSA RASADSA added the type:enhancement ✨ New feature or request label Nov 30, 2021
@tmbo
Copy link
Member

tmbo commented Dec 6, 2021

I'll update the issue according to the discussion:

  • we won't add native support for kafka
  • we will add documentation to document how to enable kafka

@tmbo tmbo added the type:docs 📖 Improvements or additions to documentation label Dec 6, 2021
@tczekajlo tczekajlo changed the title Provide option in values for enabling Kafka Document how to use deployment along with Kafka Jan 25, 2022
@andreoid
Copy link
Contributor

Open issue has been moved to JIRA: https://rasahq.atlassian.net/browse/INFRA-7

@andreoid andreoid reopened this Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:docs 📖 Improvements or additions to documentation type:enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants