Skip to content
This repository has been archived by the owner on Mar 9, 2021. It is now read-only.

Add source-kafka plugin #19

Closed
wants to merge 5 commits into from
Closed

Conversation

daisy-ycguo
Copy link
Member

The source-kafka plugin includes:

  • feature to create a kafka source
  • feature to delete a kafka source
  • tests: e2e test and unit tests

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 20, 2020
@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 20, 2020
Copy link
Member

@mattmoor mattmoor left a comment

Choose a reason for hiding this comment

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

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

Copy link
Member

@mattmoor mattmoor left a comment

Choose a reason for hiding this comment

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

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

@@ -0,0 +1,89 @@
## kn-source_kafka

`kn-source_kafka` is a plugin of Knative Client, which allows you to management of Kafka event source interactively from the command line.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why it is called kn-source_kafka and now kn-source-kafka ? I would expect a command like kn source kafka much like kn source apiserver or any other source.

Copy link
Member Author

Choose a reason for hiding this comment

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

@maximilien mentioned that currently kn command, e.g. kn source, was not allowed to be extended. That is, I'm not allowed to add a subcommand kafka to kn source. But it can be changed. We could talk about that in the meeting tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, if this doesn't work for kn we should fix it. Its a key feature to make built-in source, plugin source and dynamical, generic source the same for the user experience (as it really doesn't matter for the user how a source is implemented, the usage across all sources should be uniform).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, we need to have a way to make exception for kn source group. Will open an issue and submit a PR for this tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

See issue client issue #814

Copy link
Contributor

@maximilien maximilien left a comment

Choose a reason for hiding this comment

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

/test pull-knative-client-contrib-integration-tests

@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: daisy-ycguo, maximilien

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 21, 2020
@daisy-ycguo daisy-ycguo force-pushed the kafka branch 2 times, most recently from 888880d to ff7de36 Compare April 21, 2020 17:37
@daisy-ycguo
Copy link
Member Author

/test pull-knative-client-contrib-integration-tests

@daisy-ycguo
Copy link
Member Author

I will close this PR, refactor it and separate into two PRs for easy review.

@daisy-ycguo daisy-ycguo closed this May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants