-
Notifications
You must be signed in to change notification settings - Fork 588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Copy message before passing it to eventtype autocreate handler #7934
Copy message before passing it to eventtype autocreate handler #7934
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7934 +/- ##
==========================================
+ Coverage 69.22% 69.51% +0.29%
==========================================
Files 339 344 +5
Lines 19494 15994 -3500
==========================================
- Hits 13494 11119 -2375
+ Misses 5337 4189 -1148
- Partials 663 686 +23 ☔ View full report in Codecov by Sentry. |
/cc @Cali0707 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Thanks for catching this @creydr !
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Cali0707, creydr 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 |
/cherry-pick release-1.14 |
@creydr: new pull request created: #7937 In response to this:
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-sigs/prow repository. |
@creydr it would be great to get a regression test for this |
I see 2 options:
Objections @pierDipi? |
@creydr we already do this option in EKB, maybe it makes sense here as well? |
Currently we see the
TestEventTransformationForTriggerV1BrokerV1
e2e test failing, when theeventtype-auto-create
FF is enabled. This is because we're not copying the message at the right place.From the docs:
This means, that
m
gets read bybuffering.CopyMessage()
and thus is "used" for further usages./cherry-pick release-1.14