Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

[WIP] Individual config in KafkaChannel #1090

Closed
wants to merge 1 commit into from
Closed

[WIP] Individual config in KafkaChannel #1090

wants to merge 1 commit into from

Conversation

aliok
Copy link
Member

@aliok aliok commented Feb 11, 2022

Fixes #1005

Proposed Changes

Release Note

Docs

@knative-prow-robot knative-prow-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 11, 2022
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok

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 Feb 11, 2022
@codecov
Copy link

codecov bot commented Feb 11, 2022 •
edited
Loading

Codecov Report

Merging #1090 (1a453ce) into main (59a3a1c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1090   +/-   ##
=======================================
  Coverage   75.23%   75.23%           
=======================================
  Files         123      123           
  Lines        5726     5726           
=======================================
  Hits         4308     4308           
  Misses       1196     1196           
  Partials      222      222
Impacted Files Coverage Δ
pkg/apis/messaging/v1beta1/kafka_channel_types.go 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@knative-prow-robot
Copy link
Contributor

@aliok: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-knative-sandbox-eventing-kafka-integration-test-mt-source 1a453ce link false /test pull-knative-sandbox-eventing-kafka-integration-test-mt-source

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

matzew
matzew reviewed Feb 14, 2022
View reviewed changes
pkg/apis/messaging/v1beta1/kafka_channel_types.go
@@ -64,6 +64,22 @@ var (

// KafkaChannelSpec defines the specification for a KafkaChannel.
type KafkaChannelSpec struct {
// BootstrapServers is the Kafka Channel bootstrap servers. When not provided, the implementation will
// fall back to the configmap in the config field. If there are also no bootstrapServers defined in that,
Copy link
Contributor

Choose a reason for hiding this comment

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

for the future: do we envision to retire the global config, all together?

pierDipi
pierDipi reviewed Mar 7, 2022
View reviewed changes
Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

LGTM

can you run hack/update-codegen.sh?

pierDipi
pierDipi reviewed Mar 7, 2022
View reviewed changes
pkg/apis/messaging/v1beta1/kafka_channel_types.go
Comment on lines +77 to +81
// Config is a KReference to the configuration that specifies
// configuration options for this Channel. For example, this could be
// a pointer to a ConfigMap.
// +optional
Config *duckv1.KReference `json:"config,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

what's the use case for this?

@matzew
Copy link
Contributor

matzew commented Jun 8, 2022

@aliok with the on-going data-plane isolation, I think we should keep this proposal also in our backheads, as this allows individual configuration of the kafka channel's kafka (similar to the broker.spec.config).

Currently there is no MT usage o the channel, due to this global setting, and no individual config for kafka cluster

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

@github-actions github-actions bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 7, 2022
@github-actions
Copy link

github-actions bot commented Dec 8, 2022

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 8, 2022
@knative-prow-robot
Copy link
Contributor

This issue or pull request is stale because it has been open for 90 days with no activity.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close

/lifecycle stale

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 8, 2023
@github-actions
Copy link

github-actions bot commented Apr 9, 2023

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 9, 2023
@knative-prow-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 12, 2023
@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 13, 2023
@matzew
Copy link
Contributor

matzew commented Apr 19, 2023

/close
the channel has been moved over to the eventing-kafka-broker repo

@knative-prow knative-prow bot closed this Apr 19, 2023
@knative-prow
Copy link

knative-prow bot commented Apr 19, 2023

@matzew: Closed this PR.

In response to this:

/close
the channel has been moved over to the eventing-kafka-broker repo

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pierDipi pierDipi pierDipi left review comments

@matzew matzew matzew left review comments

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

KafkaChannel Configuration option on a per object base
4 participants
@aliok @knative-prow-robot @matzew @pierDipi