Skip to content
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

KIP-365: Make KStream.process processorSupplier parameter call by name #5571

Closed
wants to merge 1 commit into from

Conversation

joan38
Copy link
Contributor

@joan38 joan38 commented Aug 25, 2018

def process(processorSupplier: () => Processor[K, V], stateStoreNames: String*)

should be:

def process(processorSupplier: => Processor[K, V], stateStoreNames: String*)

KIP: https://cwiki.apache.org/confluence/pages/resumedraft.action?draftId=89069981&draftShareId=a21bcef4-c388-4663-9c6f-9ccceee4cb30

@joan38
Copy link
Contributor Author

joan38 commented Aug 25, 2018

Just realised that by name parameters are evaluated only once.
Which invalidates this change.

@joan38 joan38 closed this Aug 25, 2018
@joan38 joan38 deleted the by-name branch August 25, 2018 17:40
@vvcephei
Copy link
Contributor

vvcephei commented Aug 27, 2018

@joan38 By-name parameters are evaluated every time they are dereferenced. But we did actually previously discuss a change like this, and decided against it.

The main reason is that this would be ok:

myThing.process(new Processor()...)

but this is a bug:

val processor = new Processor()...
myThing.process(processor)

which is just waaay to subtle.

@vvcephei
Copy link
Contributor

requiring a 0-arity function makes it much clearer that we need the ability to generate fresh Processors at will.

@vvcephei
Copy link
Contributor

But just to reiterate, we really appreciate your work in polishing this API. Thanks again!

@joan38
Copy link
Contributor Author

joan38 commented Aug 27, 2018

https://docs.scala-lang.org/tour/by-name-parameters.html

by-value parameters have the advantage that they are evaluated only once

Anyway I went through exactly all what you explained in my head after opening the PR.

Thanks mate, I actually enjoy contributing here :)
I'm based in London right now but got a job in San Francisco, if my visa application goes through I'll be moving there.
In any case I'd be happy to meet other contributors to Kafka around a beer or 2. Just ping me!

@vvcephei
Copy link
Contributor

For sure! Myself, I'm in Austin, but I head out there a few times a year. Just let us know!

@vvcephei
Copy link
Contributor

oh, that doc says "by-value parameters", I thought we were talking about by-name parameters.

@joan38
Copy link
Contributor Author

joan38 commented Aug 27, 2018

Indeed I read it wrong xD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants