The PR should be ready. I have removed the old merge() method for 1.0.0.
On Tue, Sep 19, 2017 at 4:22 PM, Guozhang Wang wrote:
> I'd like to make an exception for this KIP if it's PR can get in before the
> the code freeze date, as it's a low risk small KIP that is unlikely to
> introduce regre
I'd like to make an exception for this KIP if it's PR can get in before the
the code freeze date, as it's a low risk small KIP that is unlikely to
introduce regression.
Guozhang
On Wed, Sep 20, 2017 at 2:01 AM, Matthias J. Sax
wrote:
> @Damian, this KIP goes into 1.1 but not 1.0, so we need to
@Damian, this KIP goes into 1.1 but not 1.0, so we need to go the
deprecation way...
I would be happy to get it into 1.0 and avoid the deprecation. But
strictly speaking, the KIP vote deadline passed already... Not sure if
there is any exception from this.
-Matthias
On 9/19/17 12:17 AM, Damian
Hi Richard,
Thanks for the KIP. Looks good, just one thing: we don't need to deprecate
StreamBuilder#merge as it has been added during this release cycle. It can
just be removed.
Thanks,
Damian
On Mon, 18 Sep 2017 at 23:22 Richard Yu wrote:
> The discussion should not stay idle. Since this iss
The discussion should not stay idle. Since this issue is so small, we
should move it into the voting phase.
On Sun, Sep 17, 2017 at 1:39 PM, Matthias J. Sax
wrote:
> Thanks for updating the KIP.
>
> You are of course right, that we internally need access to
> InternalStreamBuilder, but that shou
Thanks for updating the KIP.
You are of course right, that we internally need access to
InternalStreamBuilder, but that should not be too hard and effectively
be an internal implementation detail.
Two more comments:
the new method should be
> KStream merge(KStream stream);
and not
> KStream
Correction: When the current merge() method is called with multiple
streams, a warning will be printed (or logged), but this should not hinder
ability to read the log.
There is a missing unchecked warning suppression for the old method.
However, it is not high priority due to deprecation of the old
With regards to Xavier's comment, this practice I do no think applies to
this PR. There is not much potential here for warnings to be thrown. Note
that in StreamsBuilder's merge, their is no
@SuppressWarnings("unchecked")--indicating that warnings is sparse, if not
nonexistent.
On Sun, Sep 17, 20
KIP-202 has been changed according to the conditions of your suggestion.
On Sun, Sep 17, 2017 at 8:51 AM, Richard Yu
wrote:
> I added StreamsBuilder under the assumption that InternalStreamBuilder
> would be required to merge
> two streams. However, if that is not the case, then I would still ne
I added StreamsBuilder under the assumption that InternalStreamBuilder
would be required to merge
two streams. However, if that is not the case, then I would still need a
couple of things:
1) An InternalStreamBuilder instance to instantiate a new KStream
2) The merge_name that the merged streams
Hi Richard,
Thanks a lot for the KIP!
I have three question:
- why is the new merge() method static?
- why does the new merge() method take StreamsBuilder as a parameter?
- did you think about Xavier's comment (see the JIRA in case you did
not notice it yet) about varargs vs adding some overlo
Hi,
Please take a look at:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-
202+Move+merge%28%29+from+StreamsBuilder+to+KStream
Thanks
12 matches
Mail list logo
