Hi Josep,
Thank you for the KIP!
Specifying TaskMetadata as an interface lifts the guarantee that
equals() and hashCode() on the TaskMetadata return consistent values for
the same content. Note that the current TaskMetadata implements both
methods, so theoretically users might already rely on them.
The List interface in Java defines its equals()[1] and hashCode()[2]
methods specifically and all correct implementations must adhere to that
definition.
Maybe it makes sense to add such a guarantee to the TaskMetadata
interface on which users can rely similarly to the Java's List interface.
Regarding your question about ThreadMetadata, I had the same thought. I
would add it to the KIP unless the extended KIP risks not to be accepted
before KIP freeze for 3.0.
Best,
Bruno
[1]
https://docs.oracle.com/javase/8/docs/api/java/util/List.html#equals-java.lang.Object-
[2] https://docs.oracle.com/javase/8/docs/api/java/util/List.html#hashCode--
On 31.05.21 12:36, Josep Prat wrote:
I'm starting to wonder if we should make ThreadMetadata follow the same
path as TaskMetadata and be converted to an interface + internal
implementation. What are your thoughts?
Thanks in advance,
Josep
On Mon, May 31, 2021 at 12:10 PM Josep Prat <josep.p...@aiven.io> wrote:
Hi there,
While looking closely at the classes affected by this change, I realized
that ThreadMetadata has a reference in its public API to the soon to be
deprecated TaskMetadata. For this reason, I updated the KIP to reflect that
2 more changes are needed for this KIP:
ThreadMetadata:
- Deprecate activeTasks() and standbyTasks()
- Add getActiveTasks() and getStandbyTasks() that return the new Interface
instead.
I also wrote it on the KIP, but this thing crossed my mind, do we need to
keep source compatibility for this particular change?
I would be really grateful if you could provide any feedback on the KIP (
https://cwiki.apache.org/confluence/x/XIrOCg)
Thanks in advance,
On Fri, May 28, 2021 at 10:24 AM Josep Prat <josep.p...@aiven.io> wrote:
Hi there,
I updated the KIP page with Sophie's feedback. As she already mentioned,
the intention would be to include this KIP in release 3.0.0 so we can avoid
a deprecation cycle for the getTaskID method introduced in KIP-740, I hope
I managed to capture this in the KIP description.
Just adding the link again for convenience:
https://cwiki.apache.org/confluence/x/XIrOCg
Thanks in advance,
On Thu, May 27, 2021 at 10:08 PM Josep Prat <josep.p...@aiven.io> wrote:
Hi Sophie,
Thanks for the feedback, I'll update the KIP tomorrow with your
feedback. They are all good points, and you are right, my phrasing could be
misleading.
Best,
On Thu, May 27, 2021 at 10:02 PM Sophie Blee-Goldman
<sop...@confluent.io.invalid> wrote:
Thanks for the KIP! I'm on board with the overall proposal, just a few
comments:
1) The motivation section says
TaskMetadata should have never been a class available for the general
public, but more of an internal class
which is a bit misleading as it seems to imply that TaskMetadata itself
was
never meant to be part of the public API
at all. It might be better to phrase this as "TaskMetadata was never
intended to be a public class that a user might
need to instantiate, but rather an API for exposing metadata which is
better served as an interface" --- or something
to that effect.
2) You touch on this in a later section, but it would be good to call
out
directly in the *Public Interfaces* section that
you are proposing to remove the `public TaskId getTaskId()` method that
we
added in KIP-740. Also I just want to
note that to do so will require getting this KIP into 3.0, otherwise
we'll
need to go through a deprecation cycle for
that API. I don't anticipate this being a problem as KIP freeze is still
two weeks away, but it would be good to clarify.
3) nit: we should put the new internal implementation class under
the org.apache.kafka.streams.processor.internals
package instead of under org.apache.kafka.streams.internals. But this
is an
implementation detail and as such
doesn't need to be covered by the KIP in the first place.
- Sophie
On Thu, May 27, 2021 at 1:55 AM Josep Prat <josep.p...@aiven.io.invalid
wrote:
I deliberately picked the most conservative approach of creating a new
Interface, instead of transforming the current class into an
interface.
Feedback is most welcome!
Best,
On Thu, May 27, 2021 at 10:26 AM Josep Prat <josep.p...@aiven.io>
wrote:
Hi there,
I would like to propose KIP-744, to introduce TaskMetadata as an
interface, to keep the its implementation as internal use.
This KIP can be seen as a spin-off of KIP-740.
https://cwiki.apache.org/confluence/x/XIrOCg
Best,
--
Josep Prat
*Aiven Deutschland GmbH*
Immanuelkirchstraße 26, 10405 Berlin
Amtsgericht Charlottenburg, HRB 209739 B
Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
*m:* +491715557497
*w:* aiven.io
*e:* josep.p...@aiven.io
--
Josep Prat
*Aiven Deutschland GmbH*
Immanuelkirchstraße 26, 10405 Berlin
Amtsgericht Charlottenburg, HRB 209739 B
Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
*m:* +491715557497
*w:* aiven.io
*e:* josep.p...@aiven.io
--
Josep Prat
*Aiven Deutschland GmbH*
Immanuelkirchstraße 26, 10405 Berlin
Amtsgericht Charlottenburg, HRB 209739 B
Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
*m:* +491715557497
*w:* aiven.io
*e:* josep.p...@aiven.io
--
Josep Prat
*Aiven Deutschland GmbH*
Immanuelkirchstraße 26, 10405 Berlin
Amtsgericht Charlottenburg, HRB 209739 B
Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
*m:* +491715557497
*w:* aiven.io
*e:* josep.p...@aiven.io
--
Josep Prat
*Aiven Deutschland GmbH*
Immanuelkirchstraße 26, 10405 Berlin
Amtsgericht Charlottenburg, HRB 209739 B
Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
*m:* +491715557497
*w:* aiven.io
*e:* josep.p...@aiven.io
