Skip to content

Kafka Webhook and Documentation#1316

Merged
knative-prow-robot merged 10 commits into
knative:masterfrom
nachocano:kafka-webhook
Jun 3, 2019
Merged

Kafka Webhook and Documentation#1316
knative-prow-robot merged 10 commits into
knative:masterfrom
nachocano:kafka-webhook

Conversation

@nachocano
Copy link
Copy Markdown
Contributor

@nachocano nachocano commented May 31, 2019

Fixes #1216

Proposed Changes

  • Adding a kafka admission webhook to set proper default values to KafkaChannel COs and do validations.
  • Adding new README with documentation

Release Note

NONE

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label May 31, 2019
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 31, 2019
@nachocano
Copy link
Copy Markdown
Contributor Author

/assign @vaikas-google
/assign @matzew

@nachocano nachocano changed the title Kafka Webhook Kafka Webhook and Documentation May 31, 2019
numPartitions: 1
replicationFactor: 3
```
You can configure the number of partitions with `numPartitions`, as well as the replication factor with `replicationFactor`. If not set, both will default to `1`.
Copy link
Copy Markdown
Member

@matzew matzew Jun 3, 2019

Choose a reason for hiding this comment

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

should this doc say, how to connect to the broker ? or use as default channel ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we need better documentation for all channels. And we should put that in knative/docs.
This is a copy/paster from the current provisioner-based README, plus some minor changes.
IMO we can add that in a follow up in knative/docs. What do you think?

Comment thread contrib/kafka/config/500-webhook.yaml
Comment thread contrib/kafka/config/500-webhook.yaml Outdated
@nachocano
Copy link
Copy Markdown
Contributor Author

@matzew done with changes. If you can take another look, so that we can merge this in, and then focus on the follow ups, it'd be great.
Thanks!

@matzew
Copy link
Copy Markdown
Member

matzew commented Jun 3, 2019 via email

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2019
@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Jun 3, 2019

/lgtm
/approve

@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nachocano, vaikas-google

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

The pull request process is described here

Details 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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 3, 2019
@knative-prow-robot knative-prow-robot merged commit 39705cb into knative:master Jun 3, 2019
matzew pushed a commit to matzew/eventing that referenced this pull request Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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.

Modify kafka channel to use CRD

6 participants