Skip to content

Conversation

@itsmurugappan
Copy link
Contributor

@itsmurugappan itsmurugappan commented May 18, 2020

Fixes #2401

Proposed Changes

  • Document for Kafka binding

I have assumed that with 0.15 release , KafkaSource manifests will include the crds for KafkaBinding

Also I published kafka-publisher image into my personal repo and have used it in the example, not sure if this is the right thing.

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label May 18, 2020
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 18, 2020
@mattmoor
Copy link
Member

Could we show/include the source snippet used to build the container image you are running?

@itsmurugappan
Copy link
Contributor Author

Could we show/include the source snippet used to build the container image you are running?

Added a line for the kafka publisher, that I have used the code from here.
Will add for the event display image.

@knative-prow-robot knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 18, 2020
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 18, 2020
Copy link
Contributor

@vaikas vaikas left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Few small comments.

```
1. Deploy the Apache Kafka cluster
```
$ kubectl apply -n kafka -f kafka.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

They have to first download this file? Wonder if it would be better to have this use the here document? Or, maybe add a link to this file?
Also, the step says this will create a namespace, but it actually installs kafka?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the previous step which displays the yaml, i have added "Describe the size of your Apache Kafka installation in kafka.yaml", in the next step we apply this yaml.

@knative-prow-robot knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 18, 2020
@evankanderson
Copy link
Member

/assign @mpetason @vaikas

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 23, 2020
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 24, 2020
@vaikas
Copy link
Contributor

vaikas commented May 26, 2020

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 26, 2020
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: itsmurugappan, vaikas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit 6dc6b03 into knative:master May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document KafkaBinding

7 participants