You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Romain Manni-Bucau <rm...@gmail.com> on 2018/03/12 15:59:27 UTC
NoSuchElementException in reader.getCurrent*.
Hi guys,
why reader#getCurrent* can throw NoSuchElementException,
my understanding is that the runner will guarantee that start or advance
was called and returned true when calling getCurrent so this is a case
which shouldn't happen, no?
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
Re: NoSuchElementException in reader.getCurrent*.
Posted by Thomas Groh <tg...@google.com>.
I'm not sure what you mean, JB. getCurrent* is source-implementor visible
(and must be), and users don't need to interact with it directly via
Read/other IO transforms.
WIth regard to Eugene's point - I still am strongly in favor of telling a
source that they at least *SHOULD* throw an exception if they are called
when no element is available, and it seems reasonable to me to include that
on the documentation for the Reader
On Tue, Mar 13, 2018 at 4:53 AM Jean-Baptiste Onofré <jb...@nanthrax.net>
wrote:
> I agree. I don't think it's useful to expose getCurrent to the user.
> That's more runner related.
>
> Regards
> JB
> Le 12 mars 2018, à 11:06, Romain Manni-Bucau <rm...@gmail.com> a
> écrit:
>>
>> I agree Thomas but I kind of read it as "yes we can drop that
>> constraint". If not we should also check we are used in a thread safe
>> context etc which will likely never hit the user sdk API so why doing that
>> case a particular case? Am I missing something?
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>> 2018-03-12 17:04 GMT+01:00 Thomas Groh <tg...@google.com>:
>>
>>> If a call to `getCurrentWhatever` happens after `start` or `advance` has
>>> returned false, it's a bug in the runner, but the reader needs to be able
>>> to fail, otherwise you'll get a synthetic element that doesn't really
>>> exist. If a reader throws `NoSuchElementException` after the most recent
>>> call returned true, the reader isn't conforming to spec.
>>>
>>>
>>> On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau <
>>> rmannibucau@gmail.com> wrote:
>>>
>>>> Hi guys,
>>>>
>>>> why reader#getCurrent* can throw NoSuchElementException,
>>>> my understanding is that the runner will guarantee that start or
>>>> advance was called and returned true when calling getCurrent so this is a
>>>> case which shouldn't happen, no?
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>> <http://rmannibucau.wordpress.com> | Github
>>>> <https://github.com/rmannibucau> | LinkedIn
>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>
>>>
>>
Re: NoSuchElementException in reader.getCurrent*.
Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
I agree. I don't think it's useful to expose getCurrent to the user. That's more runner related.
Regards
JB
Le 12 mars 2018 à 11:06, à 11:06, Romain Manni-Bucau <rm...@gmail.com> a écrit:
>I agree Thomas but I kind of read it as "yes we can drop that
>constraint".
>If not we should also check we are used in a thread safe context etc
>which
>will likely never hit the user sdk API so why doing that case a
>particular
>case? Am I missing something?
>
>
>Romain Manni-Bucau
>@rmannibucau <https://twitter.com/rmannibucau> | Blog
><https://rmannibucau.metawerx.net/> | Old Blog
><http://rmannibucau.wordpress.com> | Github
><https://github.com/rmannibucau> |
>LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
><https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
>2018-03-12 17:04 GMT+01:00 Thomas Groh <tg...@google.com>:
>
>> If a call to `getCurrentWhatever` happens after `start` or `advance`
>has
>> returned false, it's a bug in the runner, but the reader needs to be
>able
>> to fail, otherwise you'll get a synthetic element that doesn't really
>> exist. If a reader throws `NoSuchElementException` after the most
>recent
>> call returned true, the reader isn't conforming to spec.
>>
>>
>> On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau
><rm...@gmail.com>
>> wrote:
>>
>>> Hi guys,
>>>
>>> why reader#getCurrent* can throw NoSuchElementException,
>>> my understanding is that the runner will guarantee that start or
>advance
>>> was called and returned true when calling getCurrent so this is a
>case
>>> which shouldn't happen, no?
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>
><https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>
>>
Re: NoSuchElementException in reader.getCurrent*.
Posted by Eugene Kirpichov <ki...@google.com>.
I think I'm actually with Romain here and I'm somewhat in favor of dropping
this.
The only entity who can violate the start/advance/getCurrent contract is
the runner or a test utility, and they have their own unit tests to ensure
they don't - there is no reason to re-enforce their contract via every IO
that uses the Source API. It's a minor point, but it does clutter the code
of every such IO a bit.
On Mon, Mar 12, 2018 at 9:40 AM Romain Manni-Bucau <rm...@gmail.com>
wrote:
> Hmm, wrong link? Iterator contract doesn't require hasNext() to be called
> compared to beam (even if it is highly recommanded). Aligned on beam it
> means the runner can acll getCurrent() without calling start or advanced
> and consider it is done if there is a NSEE which will likely never works
> since start/advance bufferizes current in most (all?) impl. So it is likely
> a bit different.
>
> To be concrete iterator often handle hasNext implicitly in next() if
> needed whereas beam IOs never do it.
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
> 2018-03-12 17:35 GMT+01:00 Thomas Groh <tg...@google.com>:
>
>> The correct sequencing and respecting return values of `start` and
>> `advance` is a precondition, and `NoSuchElementException` is the failure
>> mode if the precondition isn't met. Documenting the behavior in the case of
>> precondition failures is entirely reasonable. For example, look at Java's
>> `Iterator` documentation, which has basically the same text:
>> https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html
>>
>>
>> On Mon, Mar 12, 2018 at 9:20 AM Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>>
>>> I agree Thomas but I kind of read it as "yes we can drop that
>>> constraint". If not we should also check we are used in a thread safe
>>> context etc which will likely never hit the user sdk API so why doing that
>>> case a particular case? Am I missing something?
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>
>>> 2018-03-12 17:04 GMT+01:00 Thomas Groh <tg...@google.com>:
>>>
>>>> If a call to `getCurrentWhatever` happens after `start` or `advance`
>>>> has returned false, it's a bug in the runner, but the reader needs to be
>>>> able to fail, otherwise you'll get a synthetic element that doesn't really
>>>> exist. If a reader throws `NoSuchElementException` after the most recent
>>>> call returned true, the reader isn't conforming to spec.
>>>>
>>>>
>>>> On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau <
>>>> rmannibucau@gmail.com> wrote:
>>>>
>>>>> Hi guys,
>>>>>
>>>>> why reader#getCurrent* can throw NoSuchElementException,
>>>>> my understanding is that the runner will guarantee that start or
>>>>> advance was called and returned true when calling getCurrent so this is a
>>>>> case which shouldn't happen, no?
>>>>>
>>>>> Romain Manni-Bucau
>>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>> <http://rmannibucau.wordpress.com> | Github
>>>>> <https://github.com/rmannibucau> | LinkedIn
>>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>>
>>>>
>>>
>
Re: NoSuchElementException in reader.getCurrent*.
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hmm, wrong link? Iterator contract doesn't require hasNext() to be called
compared to beam (even if it is highly recommanded). Aligned on beam it
means the runner can acll getCurrent() without calling start or advanced
and consider it is done if there is a NSEE which will likely never works
since start/advance bufferizes current in most (all?) impl. So it is likely
a bit different.
To be concrete iterator often handle hasNext implicitly in next() if needed
whereas beam IOs never do it.
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-03-12 17:35 GMT+01:00 Thomas Groh <tg...@google.com>:
> The correct sequencing and respecting return values of `start` and
> `advance` is a precondition, and `NoSuchElementException` is the failure
> mode if the precondition isn't met. Documenting the behavior in the case of
> precondition failures is entirely reasonable. For example, look at Java's
> `Iterator` documentation, which has basically the same text:
> https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html
>
>
> On Mon, Mar 12, 2018 at 9:20 AM Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
>> I agree Thomas but I kind of read it as "yes we can drop that
>> constraint". If not we should also check we are used in a thread safe
>> context etc which will likely never hit the user sdk API so why doing that
>> case a particular case? Am I missing something?
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>> 2018-03-12 17:04 GMT+01:00 Thomas Groh <tg...@google.com>:
>>
>>> If a call to `getCurrentWhatever` happens after `start` or `advance` has
>>> returned false, it's a bug in the runner, but the reader needs to be able
>>> to fail, otherwise you'll get a synthetic element that doesn't really
>>> exist. If a reader throws `NoSuchElementException` after the most recent
>>> call returned true, the reader isn't conforming to spec.
>>>
>>>
>>> On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau <
>>> rmannibucau@gmail.com> wrote:
>>>
>>>> Hi guys,
>>>>
>>>> why reader#getCurrent* can throw NoSuchElementException,
>>>> my understanding is that the runner will guarantee that start or
>>>> advance was called and returned true when calling getCurrent so this is a
>>>> case which shouldn't happen, no?
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>> <http://rmannibucau.wordpress.com> | Github
>>>> <https://github.com/rmannibucau> | LinkedIn
>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>>
>>>
>>
Re: NoSuchElementException in reader.getCurrent*.
Posted by Thomas Groh <tg...@google.com>.
The correct sequencing and respecting return values of `start` and
`advance` is a precondition, and `NoSuchElementException` is the failure
mode if the precondition isn't met. Documenting the behavior in the case of
precondition failures is entirely reasonable. For example, look at Java's
`Iterator` documentation, which has basically the same text:
https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html
On Mon, Mar 12, 2018 at 9:20 AM Romain Manni-Bucau <rm...@gmail.com>
wrote:
> I agree Thomas but I kind of read it as "yes we can drop that constraint".
> If not we should also check we are used in a thread safe context etc which
> will likely never hit the user sdk API so why doing that case a particular
> case? Am I missing something?
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>
> 2018-03-12 17:04 GMT+01:00 Thomas Groh <tg...@google.com>:
>
>> If a call to `getCurrentWhatever` happens after `start` or `advance` has
>> returned false, it's a bug in the runner, but the reader needs to be able
>> to fail, otherwise you'll get a synthetic element that doesn't really
>> exist. If a reader throws `NoSuchElementException` after the most recent
>> call returned true, the reader isn't conforming to spec.
>>
>>
>> On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>>
>>> Hi guys,
>>>
>>> why reader#getCurrent* can throw NoSuchElementException,
>>> my understanding is that the runner will guarantee that start or advance
>>> was called and returned true when calling getCurrent so this is a case
>>> which shouldn't happen, no?
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>>
>>
>
Re: NoSuchElementException in reader.getCurrent*.
Posted by Romain Manni-Bucau <rm...@gmail.com>.
I agree Thomas but I kind of read it as "yes we can drop that constraint".
If not we should also check we are used in a thread safe context etc which
will likely never hit the user sdk API so why doing that case a particular
case? Am I missing something?
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>
2018-03-12 17:04 GMT+01:00 Thomas Groh <tg...@google.com>:
> If a call to `getCurrentWhatever` happens after `start` or `advance` has
> returned false, it's a bug in the runner, but the reader needs to be able
> to fail, otherwise you'll get a synthetic element that doesn't really
> exist. If a reader throws `NoSuchElementException` after the most recent
> call returned true, the reader isn't conforming to spec.
>
>
> On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
>> Hi guys,
>>
>> why reader#getCurrent* can throw NoSuchElementException,
>> my understanding is that the runner will guarantee that start or advance
>> was called and returned true when calling getCurrent so this is a case
>> which shouldn't happen, no?
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://rmannibucau.metawerx.net/> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github
>> <https://github.com/rmannibucau> | LinkedIn
>> <https://www.linkedin.com/in/rmannibucau> | Book
>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>
>
Re: NoSuchElementException in reader.getCurrent*.
Posted by Thomas Groh <tg...@google.com>.
If a call to `getCurrentWhatever` happens after `start` or `advance` has
returned false, it's a bug in the runner, but the reader needs to be able
to fail, otherwise you'll get a synthetic element that doesn't really
exist. If a reader throws `NoSuchElementException` after the most recent
call returned true, the reader isn't conforming to spec.
On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau <rm...@gmail.com>
wrote:
> Hi guys,
>
> why reader#getCurrent* can throw NoSuchElementException,
> my understanding is that the runner will guarantee that start or advance
> was called and returned true when calling getCurrent so this is a case
> which shouldn't happen, no?
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>