[streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Márton Balassi
Hey,

I have just come across a shortcoming of the streaming Scala API: it
completely lacks the Scala implementation of the DataStreamSink and instead
the Java version is used. [1]

I would regard this as a bug that needs a fix for 1.0.1. Unfortunately this
is also api-breaking.

Will post it to JIRA shortly - but issues.apache.org is unresponsive for me
currently. Wanted to raise the issue here as it might affect the api.

[1]
https://github.com/apache/flink/blob/master/flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala#L928-L929
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Márton Balassi
The JIRA issue is FLINK-3610.

On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <[hidden email]>
wrote:

>
> I have just come across a shortcoming of the streaming Scala API: it
> completely lacks the Scala implementation of the DataStreamSink and
> instead the Java version is used. [1]
>
> I would regard this as a bug that needs a fix for 1.0.1. Unfortunately
> this is also api-breaking.
>
> Will post it to JIRA shortly - but issues.apache.org is unresponsive for
> me currently. Wanted to raise the issue here as it might affect the api.
>
> [1] https://github.com/apache/flink/blob/master/flink-streaming-scala
> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> #L928-L929
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Robert Metzger
Hey,
JIRA was down for quite a while yesterday. Sadly, I don't think we can
merge the change because its API breaking.
One of the promises of the 1.0 release is that we are not breaking any APIs
in the 1.x.y series of Flink. We can fix those issues with a 2.x release.

On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <[hidden email]>
wrote:

> The JIRA issue is FLINK-3610.
>
> On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <[hidden email]>
> wrote:
>
> >
> > I have just come across a shortcoming of the streaming Scala API: it
> > completely lacks the Scala implementation of the DataStreamSink and
> > instead the Java version is used. [1]
> >
> > I would regard this as a bug that needs a fix for 1.0.1. Unfortunately
> > this is also api-breaking.
> >
> > Will post it to JIRA shortly - but issues.apache.org is unresponsive for
> > me currently. Wanted to raise the issue here as it might affect the api.
> >
> > [1] https://github.com/apache/flink/blob/master/flink-streaming-scala
> > /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> > #L928-L929
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Márton Balassi
Ok, if that is what we promised let's stick to that.
Then would you suggest to open a release-2.0 branch and merge it there?

On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <[hidden email]>
wrote:

> Hey,
> JIRA was down for quite a while yesterday. Sadly, I don't think we can
> merge the change because its API breaking.
> One of the promises of the 1.0 release is that we are not breaking any APIs
> in the 1.x.y series of Flink. We can fix those issues with a 2.x release.
>
> On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <[hidden email]>
> wrote:
>
> > The JIRA issue is FLINK-3610.
> >
> > On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
> [hidden email]>
> > wrote:
> >
> > >
> > > I have just come across a shortcoming of the streaming Scala API: it
> > > completely lacks the Scala implementation of the DataStreamSink and
> > > instead the Java version is used. [1]
> > >
> > > I would regard this as a bug that needs a fix for 1.0.1. Unfortunately
> > > this is also api-breaking.
> > >
> > > Will post it to JIRA shortly - but issues.apache.org is unresponsive
> for
> > > me currently. Wanted to raise the issue here as it might affect the
> api.
> > >
> > > [1] https://github.com/apache/flink/blob/master/flink-streaming-scala
> > > /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> > > #L928-L929
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Robert Metzger
I think its too early to fork off a 2.0 branch. I have absolutely no idea
when a 2.0 release becomes relevant, could be easily a year from now.

The API stability guarantees don't forbid adding new methods. Maybe we can
find a good way to resolve the issue without changing the signature of
existing methods.
And for tracking API breaking changes, maybe it makes sense to create a
2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.

On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <[hidden email]>
wrote:

> Ok, if that is what we promised let's stick to that.
> Then would you suggest to open a release-2.0 branch and merge it there?
>
> On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <[hidden email]>
> wrote:
>
> > Hey,
> > JIRA was down for quite a while yesterday. Sadly, I don't think we can
> > merge the change because its API breaking.
> > One of the promises of the 1.0 release is that we are not breaking any
> APIs
> > in the 1.x.y series of Flink. We can fix those issues with a 2.x release.
> >
> > On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
> [hidden email]>
> > wrote:
> >
> > > The JIRA issue is FLINK-3610.
> > >
> > > On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
> > [hidden email]>
> > > wrote:
> > >
> > > >
> > > > I have just come across a shortcoming of the streaming Scala API: it
> > > > completely lacks the Scala implementation of the DataStreamSink and
> > > > instead the Java version is used. [1]
> > > >
> > > > I would regard this as a bug that needs a fix for 1.0.1.
> Unfortunately
> > > > this is also api-breaking.
> > > >
> > > > Will post it to JIRA shortly - but issues.apache.org is unresponsive
> > for
> > > > me currently. Wanted to raise the issue here as it might affect the
> > api.
> > > >
> > > > [1]
> https://github.com/apache/flink/blob/master/flink-streaming-scala
> > > > /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> > > > #L928-L929
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Márton Balassi
This approach means that we will have API breaking changes lingering around
in random people's remote repos that will be untrackable in no time and
people will not be able to build on each others changes. Whoever will have
the pleasure to eventually merge those together will be on the receiving
end of some great fun. I see two alternatives: Either fork a "traditional"
release-2.0 already, which I do agree would be an overkill to maintain with
all the commits flowing in for a year or so. Or at least we could dedicate
a branch to centrally collect the api breaking changes since the
release-1.0 api.

I appreciate the api extending methodology and would certainly love to
follow it myself, but the concrete issue basically is that the return type
of a function in the public api is not right. We can methods with the
correct return type, but that is borderline more ugly than the current
solution.

On Sun, Mar 13, 2016 at 12:14 PM, Robert Metzger <[hidden email]>
wrote:

> I think its too early to fork off a 2.0 branch. I have absolutely no idea
> when a 2.0 release becomes relevant, could be easily a year from now.
>
> The API stability guarantees don't forbid adding new methods. Maybe we can
> find a good way to resolve the issue without changing the signature of
> existing methods.
> And for tracking API breaking changes, maybe it makes sense to create a
> 2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.
>
> On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <[hidden email]
> >
> wrote:
>
> > Ok, if that is what we promised let's stick to that.
> > Then would you suggest to open a release-2.0 branch and merge it there?
> >
> > On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <[hidden email]>
> > wrote:
> >
> > > Hey,
> > > JIRA was down for quite a while yesterday. Sadly, I don't think we can
> > > merge the change because its API breaking.
> > > One of the promises of the 1.0 release is that we are not breaking any
> > APIs
> > > in the 1.x.y series of Flink. We can fix those issues with a 2.x
> release.
> > >
> > > On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
> > [hidden email]>
> > > wrote:
> > >
> > > > The JIRA issue is FLINK-3610.
> > > >
> > > > On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
> > > [hidden email]>
> > > > wrote:
> > > >
> > > > >
> > > > > I have just come across a shortcoming of the streaming Scala API:
> it
> > > > > completely lacks the Scala implementation of the DataStreamSink and
> > > > > instead the Java version is used. [1]
> > > > >
> > > > > I would regard this as a bug that needs a fix for 1.0.1.
> > Unfortunately
> > > > > this is also api-breaking.
> > > > >
> > > > > Will post it to JIRA shortly - but issues.apache.org is
> unresponsive
> > > for
> > > > > me currently. Wanted to raise the issue here as it might affect the
> > > api.
> > > > >
> > > > > [1]
> > https://github.com/apache/flink/blob/master/flink-streaming-scala
> > > > >
> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> > > > > #L928-L929
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Chesnay Schepler-3
In reply to this post by Robert Metzger
On 13.03.2016 12:14, Robert Metzger wrote:
> I think its too early to fork off a 2.0 branch. I have absolutely no idea
> when a 2.0 release becomes relevant, could be easily a year from now.
at first i was going to agree with Robert, but then...I mean the issue
with not allowing breaking changes
is that effectively this means we won't work on these issues until 2.0
comes around. Since otherwise,
the contributor would have to stash that change themselves in their own
repository for god-knows how long.
Chances are that work will go to waste anyway because they forget /
delete it.

having a central place (not necessarily a separate branch, maybe a repo
with a separate branch for every commit)
where we can stash this work could prove useful; instead of starting to
work on these issues all at once for 2.0,
we could save some work by only having to rebase them in one way or another.

> And for tracking API breaking changes, maybe it makes sense to create a
> 2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.
+1 for adding a 2.0.0 version tag/. /This is the perfect use-case for it.//

>
> On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <[hidden email]>
> wrote:
>
>> Ok, if that is what we promised let's stick to that.
>> Then would you suggest to open a release-2.0 branch and merge it there?
>>
>> On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <[hidden email]>
>> wrote:
>>
>>> Hey,
>>> JIRA was down for quite a while yesterday. Sadly, I don't think we can
>>> merge the change because its API breaking.
>>> One of the promises of the 1.0 release is that we are not breaking any
>> APIs
>>> in the 1.x.y series of Flink. We can fix those issues with a 2.x release.
>>>
>>> On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
>> [hidden email]>
>>> wrote:
>>>
>>>> The JIRA issue is FLINK-3610.
>>>>
>>>> On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
>>> [hidden email]>
>>>> wrote:
>>>>
>>>>> I have just come across a shortcoming of the streaming Scala API: it
>>>>> completely lacks the Scala implementation of the DataStreamSink and
>>>>> instead the Java version is used. [1]
>>>>>
>>>>> I would regard this as a bug that needs a fix for 1.0.1.
>> Unfortunately
>>>>> this is also api-breaking.
>>>>>
>>>>> Will post it to JIRA shortly - but issues.apache.org is unresponsive
>>> for
>>>>> me currently. Wanted to raise the issue here as it might affect the
>>> api.
>>>>> [1]
>> https://github.com/apache/flink/blob/master/flink-streaming-scala
>>>>> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
>>>>> #L928-L929
>>>>>

Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Gyula Fóra-2
Hi,

I think this is an important question that will surely come up in some
cases in the future.

I see your point Robert, that we have promised api compatibility for 1.x.y
releases, but I am not sure that this should cover things that are clearly
just unintended errors in the api from our side.

I am not sure what would be the right action regarding issues like this in
the future.

Gyula

Chesnay Schepler <[hidden email]> ezt írta (időpont: 2016. márc. 13.,
V, 12:37):

> On 13.03.2016 12:14, Robert Metzger wrote:
> > I think its too early to fork off a 2.0 branch. I have absolutely no idea
> > when a 2.0 release becomes relevant, could be easily a year from now.
> at first i was going to agree with Robert, but then...I mean the issue
> with not allowing breaking changes
> is that effectively this means we won't work on these issues until 2.0
> comes around. Since otherwise,
> the contributor would have to stash that change themselves in their own
> repository for god-knows how long.
> Chances are that work will go to waste anyway because they forget /
> delete it.
>
> having a central place (not necessarily a separate branch, maybe a repo
> with a separate branch for every commit)
> where we can stash this work could prove useful; instead of starting to
> work on these issues all at once for 2.0,
> we could save some work by only having to rebase them in one way or
> another.
>
> > And for tracking API breaking changes, maybe it makes sense to create a
> > 2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.
> +1 for adding a 2.0.0 version tag/. /This is the perfect use-case for it.//
> >
> > On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <
> [hidden email]>
> > wrote:
> >
> >> Ok, if that is what we promised let's stick to that.
> >> Then would you suggest to open a release-2.0 branch and merge it there?
> >>
> >> On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <[hidden email]>
> >> wrote:
> >>
> >>> Hey,
> >>> JIRA was down for quite a while yesterday. Sadly, I don't think we can
> >>> merge the change because its API breaking.
> >>> One of the promises of the 1.0 release is that we are not breaking any
> >> APIs
> >>> in the 1.x.y series of Flink. We can fix those issues with a 2.x
> release.
> >>>
> >>> On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
> >> [hidden email]>
> >>> wrote:
> >>>
> >>>> The JIRA issue is FLINK-3610.
> >>>>
> >>>> On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
> >>> [hidden email]>
> >>>> wrote:
> >>>>
> >>>>> I have just come across a shortcoming of the streaming Scala API: it
> >>>>> completely lacks the Scala implementation of the DataStreamSink and
> >>>>> instead the Java version is used. [1]
> >>>>>
> >>>>> I would regard this as a bug that needs a fix for 1.0.1.
> >> Unfortunately
> >>>>> this is also api-breaking.
> >>>>>
> >>>>> Will post it to JIRA shortly - but issues.apache.org is unresponsive
> >>> for
> >>>>> me currently. Wanted to raise the issue here as it might affect the
> >>> api.
> >>>>> [1]
> >> https://github.com/apache/flink/blob/master/flink-streaming-scala
> >>>>> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> >>>>> #L928-L929
> >>>>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Fabian Hueske-2
Yes, we will have more of these issues in the future and each issue will
need a separate discussion.
I don't think that clearly unintended errors (I hope we won't find any
intended errors) are a sufficient reason to break stable a stable API.
IMO, the question that needs to be answered how much of an issue it is (put
it on a scale: bug > limitation > inconsistent API) and whether there are
workarounds that avoid API breaking changes.

Cheers, Fabian



2016-03-13 19:06 GMT+01:00 Gyula Fóra <[hidden email]>:

> Hi,
>
> I think this is an important question that will surely come up in some
> cases in the future.
>
> I see your point Robert, that we have promised api compatibility for 1.x.y
> releases, but I am not sure that this should cover things that are clearly
> just unintended errors in the api from our side.
>
> I am not sure what would be the right action regarding issues like this in
> the future.
>
> Gyula
>
> Chesnay Schepler <[hidden email]> ezt írta (időpont: 2016. márc. 13.,
> V, 12:37):
>
> > On 13.03.2016 12:14, Robert Metzger wrote:
> > > I think its too early to fork off a 2.0 branch. I have absolutely no
> idea
> > > when a 2.0 release becomes relevant, could be easily a year from now.
> > at first i was going to agree with Robert, but then...I mean the issue
> > with not allowing breaking changes
> > is that effectively this means we won't work on these issues until 2.0
> > comes around. Since otherwise,
> > the contributor would have to stash that change themselves in their own
> > repository for god-knows how long.
> > Chances are that work will go to waste anyway because they forget /
> > delete it.
> >
> > having a central place (not necessarily a separate branch, maybe a repo
> > with a separate branch for every commit)
> > where we can stash this work could prove useful; instead of starting to
> > work on these issues all at once for 2.0,
> > we could save some work by only having to rebase them in one way or
> > another.
> >
> > > And for tracking API breaking changes, maybe it makes sense to create a
> > > 2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.
> > +1 for adding a 2.0.0 version tag/. /This is the perfect use-case for
> it.//
> > >
> > > On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <
> > [hidden email]>
> > > wrote:
> > >
> > >> Ok, if that is what we promised let's stick to that.
> > >> Then would you suggest to open a release-2.0 branch and merge it
> there?
> > >>
> > >> On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <[hidden email]
> >
> > >> wrote:
> > >>
> > >>> Hey,
> > >>> JIRA was down for quite a while yesterday. Sadly, I don't think we
> can
> > >>> merge the change because its API breaking.
> > >>> One of the promises of the 1.0 release is that we are not breaking
> any
> > >> APIs
> > >>> in the 1.x.y series of Flink. We can fix those issues with a 2.x
> > release.
> > >>>
> > >>> On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
> > >> [hidden email]>
> > >>> wrote:
> > >>>
> > >>>> The JIRA issue is FLINK-3610.
> > >>>>
> > >>>> On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
> > >>> [hidden email]>
> > >>>> wrote:
> > >>>>
> > >>>>> I have just come across a shortcoming of the streaming Scala API:
> it
> > >>>>> completely lacks the Scala implementation of the DataStreamSink and
> > >>>>> instead the Java version is used. [1]
> > >>>>>
> > >>>>> I would regard this as a bug that needs a fix for 1.0.1.
> > >> Unfortunately
> > >>>>> this is also api-breaking.
> > >>>>>
> > >>>>> Will post it to JIRA shortly - but issues.apache.org is
> unresponsive
> > >>> for
> > >>>>> me currently. Wanted to raise the issue here as it might affect the
> > >>> api.
> > >>>>> [1]
> > >> https://github.com/apache/flink/blob/master/flink-streaming-scala
> > >>>>>
> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> > >>>>> #L928-L929
> > >>>>>
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Aljoscha Krettek-2
By the way, I don’t think it’s a bug that addSink() returns the Java DataStreamSink. Having a Scala specific version of a DataStreamSink would not add functionality in this place, just code bloat.

> On 14 Mar 2016, at 10:05, Fabian Hueske <[hidden email]> wrote:
>
> Yes, we will have more of these issues in the future and each issue will
> need a separate discussion.
> I don't think that clearly unintended errors (I hope we won't find any
> intended errors) are a sufficient reason to break stable a stable API.
> IMO, the question that needs to be answered how much of an issue it is (put
> it on a scale: bug > limitation > inconsistent API) and whether there are
> workarounds that avoid API breaking changes.
>
> Cheers, Fabian
>
>
>
> 2016-03-13 19:06 GMT+01:00 Gyula Fóra <[hidden email]>:
>
>> Hi,
>>
>> I think this is an important question that will surely come up in some
>> cases in the future.
>>
>> I see your point Robert, that we have promised api compatibility for 1.x.y
>> releases, but I am not sure that this should cover things that are clearly
>> just unintended errors in the api from our side.
>>
>> I am not sure what would be the right action regarding issues like this in
>> the future.
>>
>> Gyula
>>
>> Chesnay Schepler <[hidden email]> ezt írta (időpont: 2016. márc. 13.,
>> V, 12:37):
>>
>>> On 13.03.2016 12:14, Robert Metzger wrote:
>>>> I think its too early to fork off a 2.0 branch. I have absolutely no
>> idea
>>>> when a 2.0 release becomes relevant, could be easily a year from now.
>>> at first i was going to agree with Robert, but then...I mean the issue
>>> with not allowing breaking changes
>>> is that effectively this means we won't work on these issues until 2.0
>>> comes around. Since otherwise,
>>> the contributor would have to stash that change themselves in their own
>>> repository for god-knows how long.
>>> Chances are that work will go to waste anyway because they forget /
>>> delete it.
>>>
>>> having a central place (not necessarily a separate branch, maybe a repo
>>> with a separate branch for every commit)
>>> where we can stash this work could prove useful; instead of starting to
>>> work on these issues all at once for 2.0,
>>> we could save some work by only having to rebase them in one way or
>>> another.
>>>
>>>> And for tracking API breaking changes, maybe it makes sense to create a
>>>> 2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.
>>> +1 for adding a 2.0.0 version tag/. /This is the perfect use-case for
>> it.//
>>>>
>>>> On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <
>>> [hidden email]>
>>>> wrote:
>>>>
>>>>> Ok, if that is what we promised let's stick to that.
>>>>> Then would you suggest to open a release-2.0 branch and merge it
>> there?
>>>>>
>>>>> On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <[hidden email]
>>>
>>>>> wrote:
>>>>>
>>>>>> Hey,
>>>>>> JIRA was down for quite a while yesterday. Sadly, I don't think we
>> can
>>>>>> merge the change because its API breaking.
>>>>>> One of the promises of the 1.0 release is that we are not breaking
>> any
>>>>> APIs
>>>>>> in the 1.x.y series of Flink. We can fix those issues with a 2.x
>>> release.
>>>>>>
>>>>>> On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
>>>>> [hidden email]>
>>>>>> wrote:
>>>>>>
>>>>>>> The JIRA issue is FLINK-3610.
>>>>>>>
>>>>>>> On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
>>>>>> [hidden email]>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> I have just come across a shortcoming of the streaming Scala API:
>> it
>>>>>>>> completely lacks the Scala implementation of the DataStreamSink and
>>>>>>>> instead the Java version is used. [1]
>>>>>>>>
>>>>>>>> I would regard this as a bug that needs a fix for 1.0.1.
>>>>> Unfortunately
>>>>>>>> this is also api-breaking.
>>>>>>>>
>>>>>>>> Will post it to JIRA shortly - but issues.apache.org is
>> unresponsive
>>>>>> for
>>>>>>>> me currently. Wanted to raise the issue here as it might affect the
>>>>>> api.
>>>>>>>> [1]
>>>>> https://github.com/apache/flink/blob/master/flink-streaming-scala
>>>>>>>>
>> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
>>>>>>>> #L928-L929
>>>>>>>>
>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Till Rohrmann
I agree with Aljoscha on this one, because `DataStreamSink` only contains
setters which are compatible with the Scala API.

On Mon, Mar 14, 2016 at 11:02 AM, Aljoscha Krettek <[hidden email]>
wrote:

> By the way, I don’t think it’s a bug that addSink() returns the Java
> DataStreamSink. Having a Scala specific version of a DataStreamSink would
> not add functionality in this place, just code bloat.
> > On 14 Mar 2016, at 10:05, Fabian Hueske <[hidden email]> wrote:
> >
> > Yes, we will have more of these issues in the future and each issue will
> > need a separate discussion.
> > I don't think that clearly unintended errors (I hope we won't find any
> > intended errors) are a sufficient reason to break stable a stable API.
> > IMO, the question that needs to be answered how much of an issue it is
> (put
> > it on a scale: bug > limitation > inconsistent API) and whether there are
> > workarounds that avoid API breaking changes.
> >
> > Cheers, Fabian
> >
> >
> >
> > 2016-03-13 19:06 GMT+01:00 Gyula Fóra <[hidden email]>:
> >
> >> Hi,
> >>
> >> I think this is an important question that will surely come up in some
> >> cases in the future.
> >>
> >> I see your point Robert, that we have promised api compatibility for
> 1.x.y
> >> releases, but I am not sure that this should cover things that are
> clearly
> >> just unintended errors in the api from our side.
> >>
> >> I am not sure what would be the right action regarding issues like this
> in
> >> the future.
> >>
> >> Gyula
> >>
> >> Chesnay Schepler <[hidden email]> ezt írta (időpont: 2016. márc.
> 13.,
> >> V, 12:37):
> >>
> >>> On 13.03.2016 12:14, Robert Metzger wrote:
> >>>> I think its too early to fork off a 2.0 branch. I have absolutely no
> >> idea
> >>>> when a 2.0 release becomes relevant, could be easily a year from now.
> >>> at first i was going to agree with Robert, but then...I mean the issue
> >>> with not allowing breaking changes
> >>> is that effectively this means we won't work on these issues until 2.0
> >>> comes around. Since otherwise,
> >>> the contributor would have to stash that change themselves in their own
> >>> repository for god-knows how long.
> >>> Chances are that work will go to waste anyway because they forget /
> >>> delete it.
> >>>
> >>> having a central place (not necessarily a separate branch, maybe a repo
> >>> with a separate branch for every commit)
> >>> where we can stash this work could prove useful; instead of starting to
> >>> work on these issues all at once for 2.0,
> >>> we could save some work by only having to rebase them in one way or
> >>> another.
> >>>
> >>>> And for tracking API breaking changes, maybe it makes sense to create
> a
> >>>> 2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.
> >>> +1 for adding a 2.0.0 version tag/. /This is the perfect use-case for
> >> it.//
> >>>>
> >>>> On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <
> >>> [hidden email]>
> >>>> wrote:
> >>>>
> >>>>> Ok, if that is what we promised let's stick to that.
> >>>>> Then would you suggest to open a release-2.0 branch and merge it
> >> there?
> >>>>>
> >>>>> On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <
> [hidden email]
> >>>
> >>>>> wrote:
> >>>>>
> >>>>>> Hey,
> >>>>>> JIRA was down for quite a while yesterday. Sadly, I don't think we
> >> can
> >>>>>> merge the change because its API breaking.
> >>>>>> One of the promises of the 1.0 release is that we are not breaking
> >> any
> >>>>> APIs
> >>>>>> in the 1.x.y series of Flink. We can fix those issues with a 2.x
> >>> release.
> >>>>>>
> >>>>>> On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
> >>>>> [hidden email]>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> The JIRA issue is FLINK-3610.
> >>>>>>>
> >>>>>>> On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
> >>>>>> [hidden email]>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> I have just come across a shortcoming of the streaming Scala API:
> >> it
> >>>>>>>> completely lacks the Scala implementation of the DataStreamSink
> and
> >>>>>>>> instead the Java version is used. [1]
> >>>>>>>>
> >>>>>>>> I would regard this as a bug that needs a fix for 1.0.1.
> >>>>> Unfortunately
> >>>>>>>> this is also api-breaking.
> >>>>>>>>
> >>>>>>>> Will post it to JIRA shortly - but issues.apache.org is
> >> unresponsive
> >>>>>> for
> >>>>>>>> me currently. Wanted to raise the issue here as it might affect
> the
> >>>>>> api.
> >>>>>>>> [1]
> >>>>> https://github.com/apache/flink/blob/master/flink-streaming-scala
> >>>>>>>>
> >> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> >>>>>>>> #L928-L929
> >>>>>>>>
> >>>
> >>>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

stefanobaghino
+1 for considering this as a bug.

However, I do realize that API compatibility is extremely important when
you commit to it, even if present behavior is not ideal.

Silly idea (which is only applicable to Scala APIs, unfortunately): I'm
currently working on a set of API extensions to solve FLINK-1159 (accepting
case-style functions on the Scala API); to add this functionality without
breaking the APIs for the upcoming (now just past) 1.0.0 release, Till and
Stephan proposed to expose these extra methods as an implicit over the
current DataSet/DataStream API that the user can opt-in to by importing a
specific package. Thanks to Till we came up with a fairly good design,
where individual extensions can be brought in a-là-carte.

I think this mechanism can come in handy to extend the API without breaking
the compatibility, acting as a staging area to get some feedback from the
community and evaluate further improvements. Maybe it can be useful to fix
this issue. It would also remove the need of a branch for the (presumably
distant) 2.0 release.

You can find the ongoing work on this PR
<https://github.com/apache/flink/pull/1704> and decide if this approach can
be interesting for this issue.

I hope I've been helpful.

Best,
Stefano

On Mon, Mar 14, 2016 at 12:22 PM, Till Rohrmann <[hidden email]>
wrote:

> I agree with Aljoscha on this one, because `DataStreamSink` only contains
> setters which are compatible with the Scala API.
>
> On Mon, Mar 14, 2016 at 11:02 AM, Aljoscha Krettek <[hidden email]>
> wrote:
>
> > By the way, I don’t think it’s a bug that addSink() returns the Java
> > DataStreamSink. Having a Scala specific version of a DataStreamSink would
> > not add functionality in this place, just code bloat.
> > > On 14 Mar 2016, at 10:05, Fabian Hueske <[hidden email]> wrote:
> > >
> > > Yes, we will have more of these issues in the future and each issue
> will
> > > need a separate discussion.
> > > I don't think that clearly unintended errors (I hope we won't find any
> > > intended errors) are a sufficient reason to break stable a stable API.
> > > IMO, the question that needs to be answered how much of an issue it is
> > (put
> > > it on a scale: bug > limitation > inconsistent API) and whether there
> are
> > > workarounds that avoid API breaking changes.
> > >
> > > Cheers, Fabian
> > >
> > >
> > >
> > > 2016-03-13 19:06 GMT+01:00 Gyula Fóra <[hidden email]>:
> > >
> > >> Hi,
> > >>
> > >> I think this is an important question that will surely come up in some
> > >> cases in the future.
> > >>
> > >> I see your point Robert, that we have promised api compatibility for
> > 1.x.y
> > >> releases, but I am not sure that this should cover things that are
> > clearly
> > >> just unintended errors in the api from our side.
> > >>
> > >> I am not sure what would be the right action regarding issues like
> this
> > in
> > >> the future.
> > >>
> > >> Gyula
> > >>
> > >> Chesnay Schepler <[hidden email]> ezt írta (időpont: 2016. márc.
> > 13.,
> > >> V, 12:37):
> > >>
> > >>> On 13.03.2016 12:14, Robert Metzger wrote:
> > >>>> I think its too early to fork off a 2.0 branch. I have absolutely no
> > >> idea
> > >>>> when a 2.0 release becomes relevant, could be easily a year from
> now.
> > >>> at first i was going to agree with Robert, but then...I mean the
> issue
> > >>> with not allowing breaking changes
> > >>> is that effectively this means we won't work on these issues until
> 2.0
> > >>> comes around. Since otherwise,
> > >>> the contributor would have to stash that change themselves in their
> own
> > >>> repository for god-knows how long.
> > >>> Chances are that work will go to waste anyway because they forget /
> > >>> delete it.
> > >>>
> > >>> having a central place (not necessarily a separate branch, maybe a
> repo
> > >>> with a separate branch for every commit)
> > >>> where we can stash this work could prove useful; instead of starting
> to
> > >>> work on these issues all at once for 2.0,
> > >>> we could save some work by only having to rebase them in one way or
> > >>> another.
> > >>>
> > >>>> And for tracking API breaking changes, maybe it makes sense to
> create
> > a
> > >>>> 2.0.0 version in JIRA and set the "fix-for" for the issue to 2.0.
> > >>> +1 for adding a 2.0.0 version tag/. /This is the perfect use-case for
> > >> it.//
> > >>>>
> > >>>> On Sun, Mar 13, 2016 at 12:08 PM, Márton Balassi <
> > >>> [hidden email]>
> > >>>> wrote:
> > >>>>
> > >>>>> Ok, if that is what we promised let's stick to that.
> > >>>>> Then would you suggest to open a release-2.0 branch and merge it
> > >> there?
> > >>>>>
> > >>>>> On Sun, Mar 13, 2016 at 11:43 AM, Robert Metzger <
> > [hidden email]
> > >>>
> > >>>>> wrote:
> > >>>>>
> > >>>>>> Hey,
> > >>>>>> JIRA was down for quite a while yesterday. Sadly, I don't think we
> > >> can
> > >>>>>> merge the change because its API breaking.
> > >>>>>> One of the promises of the 1.0 release is that we are not breaking
> > >> any
> > >>>>> APIs
> > >>>>>> in the 1.x.y series of Flink. We can fix those issues with a 2.x
> > >>> release.
> > >>>>>>
> > >>>>>> On Sun, Mar 13, 2016 at 5:27 AM, Márton Balassi <
> > >>>>> [hidden email]>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> The JIRA issue is FLINK-3610.
> > >>>>>>>
> > >>>>>>> On Sat, Mar 12, 2016 at 8:39 PM, Márton Balassi <
> > >>>>>> [hidden email]>
> > >>>>>>> wrote:
> > >>>>>>>
> > >>>>>>>> I have just come across a shortcoming of the streaming Scala
> API:
> > >> it
> > >>>>>>>> completely lacks the Scala implementation of the DataStreamSink
> > and
> > >>>>>>>> instead the Java version is used. [1]
> > >>>>>>>>
> > >>>>>>>> I would regard this as a bug that needs a fix for 1.0.1.
> > >>>>> Unfortunately
> > >>>>>>>> this is also api-breaking.
> > >>>>>>>>
> > >>>>>>>> Will post it to JIRA shortly - but issues.apache.org is
> > >> unresponsive
> > >>>>>> for
> > >>>>>>>> me currently. Wanted to raise the issue here as it might affect
> > the
> > >>>>>> api.
> > >>>>>>>> [1]
> > >>>>> https://github.com/apache/flink/blob/master/flink-streaming-scala
> > >>>>>>>>
> > >> /src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
> > >>>>>>>> #L928-L929
> > >>>>>>>>
> > >>>
> > >>>
> > >>
> >
> >
>



--
BR,
Stefano Baghino

Software Engineer @ Radicalbit
Reply | Threaded
Open this post in threaded view
|

Re: [streaming, scala] Scala DataStream#addSink returns Java DataStreamSink

Márton Balassi
Hey,

I think we came to the agreement that this PR is not mergeable right now,
so I am closing it. I personally find it inconsistent to not have the fully
API mirrored in Scala though, but this is something that we can revisit
when prepairing 2.0.

Best,

Marton

On Mon, Mar 14, 2016 at 8:14 PM, Stefano Baghino <
[hidden email]> wrote:

>