You are viewing a plain text version of this content. The canonical link for it is here.
Posted to devnull@infra.apache.org by "joan38 (GitHub)" <gi...@apache.org> on 2018/08/25 17:16:59 UTC

[GitHub] [kafka] joan38 opened pull request #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

```
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

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] joan38 commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "joan38 (GitHub)" <gi...@apache.org>.
Indeed I read it wrong xD

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] vvcephei commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "vvcephei (GitHub)" <gi...@apache.org>.
@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()...)

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] vvcephei commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "vvcephei (GitHub)" <gi...@apache.org>.
requiring a 0-arity function makes it much clearer that we need the ability to generate fresh Processors at will.

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] vvcephei commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "vvcephei (GitHub)" <gi...@apache.org>.
oh, that doc says "by-value parameters", I thought we were talking about by-name parameters.

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] joan38 closed pull request #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "joan38 (GitHub)" <gi...@apache.org>.
[ pull request closed by joan38 ]

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] vvcephei commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "vvcephei (GitHub)" <gi...@apache.org>.
But just to reiterate, we really appreciate your work in polishing this API. Thanks again!

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] vvcephei commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "vvcephei (GitHub)" <gi...@apache.org>.
@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.

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] joan38 commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "joan38 (GitHub)" <gi...@apache.org>.
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!

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] joan38 commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "joan38 (GitHub)" <gi...@apache.org>.
Just realised that by name parameters are evaluated only once.
Which invalidates this change.

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [kafka] vvcephei commented on issue #5571: KIP-365: Make KStream.process processorSupplier parameter call by name

Posted by "vvcephei (GitHub)" <gi...@apache.org>.
For sure! Myself, I'm in Austin, but I head out there a few times a year. Just let us know!

[ Full content available at: https://github.com/apache/kafka/pull/5571 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org