-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Master Build Fixes - AclAuthorizer and TransformValues Replacement #10645
base: master
Are you sure you want to change the base?
Conversation
🎉 All Contributor License Agreements have been signed. Ready to merge. |
stream.peek((k, v) -> { }); | ||
return stream.processValues(() -> new AddKeyAndPseudoColumnsProcessor<>( | ||
keyGenerator, pseudoColumnVersion, streamSource.getSourceSchema().headers()), | ||
Named.as("KSTREAM-TRANSFORMVALUES-0000000001")); |
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.
Why is this node named explicitly?
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.
I have done as per test case requirement. I have asked kstream team if this looks okay. It was generating KSTREAM-PROCESSVALUES-0000000001
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.
Processor
name by themself should actually not be critical if they change. We need to worry about topic and state store names though.
What break if the processor name changes? If it's really only the name, it should be ok to accept the change and update the tests. If it has side-effects, setting the name here would sound rights.
The other question is, if we name explicitly, could it cause side-effect with regard to auto-naming of downstream processor -- if we name expliclity, we might not us the next index, and create different name later, what could also lead to undesried side effects.
Description
Testing done
Describe the testing strategy. Unit and integration tests are expected for any behavior changes.
Reviewer checklist