Fix KafkaSink structured content mode #2466

pierDipi · 2022-08-04T12:52:36Z

contentMode wasn't passed to the contract, so
the data plane would produce binary events regardless of
the provided contentMode.

Signed-off-by: Pierangelo Di Pilato pierdipi@redhat.com

Proposed Changes

Pass content mode to contract in KafkaSink reconciler

Release Note

Specify a different `contentMode` on KafkaSink objects wasn't having any effect, now structured or binary content mode are properly handled.

`contentMode` wasn't passed to the contract, so the data plane would produce binary events regardless of the provided contentMode. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

pierDipi · 2022-08-04T12:52:50Z

/cherry-pick release-1.6

knative-prow-robot · 2022-08-04T12:52:52Z

@pierDipi: once the present PR merges, I will cherry-pick it on top of release-1.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.6

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.

pierDipi · 2022-08-04T12:52:53Z

/cherry-pick release-1.5

knative-prow-robot · 2022-08-04T12:52:55Z

@pierDipi: once the present PR merges, I will cherry-pick it on top of release-1.5 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.5

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.

pierDipi · 2022-08-04T12:52:57Z

/cherry-pick release-1.4

knative-prow-robot · 2022-08-04T12:52:58Z

@pierDipi: once the present PR merges, I will cherry-pick it on top of release-1.4 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.4

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.

pierDipi · 2022-08-04T12:53:19Z

/assign @matzew @aliok

codecov · 2022-08-04T12:58:11Z

Codecov Report

Merging #2466 (183bd98) into main (2412c54) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #2466      +/-   ##
============================================
+ Coverage     64.59%   64.61%   +0.02%     
- Complexity      708      709       +1     
============================================
  Files           146      146              
  Lines          9803     9804       +1     
  Branches        223      223              
============================================
+ Hits           6332     6335       +3     
+ Misses         3042     3037       -5     
- Partials        429      432       +3

Flag	Coverage Δ
java-unittests	`81.40% <ø> (+0.20%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
control-plane/pkg/reconciler/sink/kafka_sink.go	`73.58% <100.00%> (+0.10%)`	⬆️
...lane/pkg/reconciler/consumergroup/consumergroup.go	`69.49% <0.00%> (-1.16%)`	⬇️
...broker/dispatcher/impl/consumer/OffsetManager.java	`95.69% <0.00%> (+1.07%)`	⬆️
...dispatcher/impl/consumer/BaseConsumerVerticle.java	`93.54% <0.00%> (+6.45%)`	⬆️
...ive/eventing/kafka/broker/core/AsyncCloseable.java	`80.00% <0.00%> (+13.33%)`	⬆️

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

pierDipi · 2022-08-04T13:47:27Z

/retest-required

matzew

/lgtm
/approve

knative-prow · 2022-08-04T14:36:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, pierDipi

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:

~~OWNERS~~ [matzew,pierDipi]

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 · 2022-08-04T15:38:04Z

@pierDipi: new pull request created: #2467

In response to this:

/cherry-pick release-1.6

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 · 2022-08-04T15:38:40Z

@pierDipi: #2466 failed to apply on top of branch "release-1.5":

Applying: Fix KafkaSink structured content mode
Using index info to reconstruct a base tree...
M	control-plane/pkg/reconciler/sink/kafka_sink.go
M	control-plane/pkg/reconciler/sink/kafka_sink_test.go
Falling back to patching base and 3-way merge...
Auto-merging control-plane/pkg/reconciler/sink/kafka_sink_test.go
Auto-merging control-plane/pkg/reconciler/sink/kafka_sink.go
CONFLICT (content): Merge conflict in control-plane/pkg/reconciler/sink/kafka_sink.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix KafkaSink structured content mode
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.5

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 · 2022-08-04T15:39:15Z

@pierDipi: #2466 failed to apply on top of branch "release-1.4":

Applying: Fix KafkaSink structured content mode
Using index info to reconstruct a base tree...
M	control-plane/pkg/reconciler/sink/kafka_sink.go
M	control-plane/pkg/reconciler/sink/kafka_sink_test.go
Falling back to patching base and 3-way merge...
Auto-merging control-plane/pkg/reconciler/sink/kafka_sink_test.go
Auto-merging control-plane/pkg/reconciler/sink/kafka_sink.go
CONFLICT (content): Merge conflict in control-plane/pkg/reconciler/sink/kafka_sink.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix KafkaSink structured content mode
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.4

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.

pierDipi · 2022-08-09T07:56:51Z

This regression was introduced in 1.6, so 1.5 and 1.4 are already working, backport for 1.6 done and released.

Fix KafkaSink structured content mode

183bd98

`contentMode` wasn't passed to the contract, so the data plane would produce binary events regardless of the provided contentMode. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

knative-prow bot added area/control-plane approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 4, 2022

knative-prow bot assigned aliok and matzew Aug 4, 2022

matzew approved these changes Aug 4, 2022

View reviewed changes

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2022

knative-prow bot merged commit 3e5468c into knative-extensions:main Aug 4, 2022

knative-prow-robot mentioned this pull request Aug 4, 2022

[release-1.6] Fix KafkaSink structured content mode #2467

Merged

pierDipi deleted the pass-content-mode branch August 9, 2022 07:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix KafkaSink structured content mode #2466

Fix KafkaSink structured content mode #2466

pierDipi commented Aug 4, 2022

pierDipi commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 4, 2022

codecov bot commented Aug 4, 2022 •

edited

pierDipi commented Aug 4, 2022

matzew left a comment

knative-prow bot commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 9, 2022

Fix KafkaSink structured content mode #2466

Fix KafkaSink structured content mode #2466

Conversation

pierDipi commented Aug 4, 2022

Proposed Changes

pierDipi commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 4, 2022

codecov bot commented Aug 4, 2022 • edited

Codecov Report

pierDipi commented Aug 4, 2022

matzew left a comment

Choose a reason for hiding this comment

knative-prow bot commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

knative-prow-robot commented Aug 4, 2022

pierDipi commented Aug 9, 2022

codecov bot commented Aug 4, 2022 •

edited