Re: why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-31 Thread Romain Manni-Bucau 
Yep but makes one other step to work in beam - dont forget you already
passed like 10 steps when you end up on coders.

My view was that the skip first close was a win-win for beam and users bit
technically you are right, users can always do it themselves.

Le 1 févr. 2018 07:16, "Lukasz Cwik"  a écrit :

> I'm not sure what you mean by it closes the door since as the caller of
> the library you can create a wrapper filter input stream that ignores close
> calls effectively overriding what happens in the UnownedInputStream.
>
> On Wed, Jan 31, 2018 at 10:08 PM, Romain Manni-Bucau <
> rmannibu...@gmail.com> wrote:
>
>>
>>
>> Le 1 févr. 2018 03:10, "Lukasz Cwik"  a écrit :
>>
>> Yes, people will write bad coders which is why this is there. No, I don't
>> think swallowing one close is what we want.
>>
>> In the case where you wants to pass an input/output stream to a library
>> that incorrectly assumes ownership, the input/output stream should be
>> wrapped right before the call to the library with a filter input/output
>> stream that swallows the close and not propagate ignoring this bad behavior
>> elsewhere.
>>
>>
>> Hmm,
>>
>> Elsewhere is nowhere else here since it wouldnt have any side effect
>> except not enforcing another layer and making smoothly work for most
>> mappers.
>>
>> Anyway I can live with it but I'm a bit sad it closes the door to the
>> easyness to write extensions.
>>
>>
>> On Wed, Jan 31, 2018 at 12:04 PM, Romain Manni-Bucau <
>> rmannibu...@gmail.com> wrote:
>>
>>> Hmm, here we are the ones owning the call since it is in a coder, no? Do
>>> we assume people will badly implement coders? In this particular case we
>>> can assume close() will be called by a framework I think.
>>> What about swallowing one close() and fail on the second?
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau  |  Blog
>>>  | Old Blog
>>>  | Github
>>>  | LinkedIn
>>> 
>>>
>>> 2018-01-31 20:59 GMT+01:00 Lukasz Cwik :
>>>
 Because people write code like:
 myMethod(InputStream in) {
   InputStream child = new InputStream(in);
   child.close();
 }

 InputStream in = new FileInputStream(... path ...);
 myMethod(in);
 myMethod(in);

 An exception will be thrown when the second myMethod call occurs.

 Unfortunately not everyone wraps their calls to a coder with an
 UnownedInputStream or a filter input stream which drops close() calls is
 why its a problem and in the few places it is done it is used to prevent
 bugs from creeping in.



 On Tue, Jan 30, 2018 at 11:29 AM, Romain Manni-Bucau <
 rmannibu...@gmail.com> wrote:

> I get the issue but I don't get the last part. Concretely we can
> support any lib by just removing the exception in the close, no? What 
> would
> be the issue? No additional wrapper, no lib integration issue.
>
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github
>  | LinkedIn
> 
>
> 2018-01-30 19:29 GMT+01:00 Lukasz Cwik :
>
>> Its common in the code base that input and output streams are passed
>> around and the caller is responsible for closing it, not the callee. The
>> UnownedInputStream is to guard against libraries that are poorly behaved
>> and assume they get ownership of the stream when it is given to them.
>>
>> In the code:
>> myMethod(InputStream in) {
>>   InputStream child = new InputStream(in);
>>   child.close();
>> }
>>
>> InputStream in = ...
>> myMethod(in);
>> myMethod(in);
>> When should "in" be closed?
>>
>> To get around this issue, create a filter input/output stream that
>> ignores close calls like on the JAXB coder:
>> https://github.com/apache/beam/blob/master/sdks/java/io/xml/
>> src/main/java/org/apache/beam/sdk/io/xml/JAXBCoder.java#L181
>>
>> We can instead swap around this pattern so that the caller guards
>> against callees closing by wrapping with a filter input/output stream but
>> this costs an additional method call for each input/output stream call.
>>
>>
>> On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau <
>> rmannibu...@gmail.com> wrote:
>>
>>> Hi guys,
>>>
>>> All is in the subject ;)
>>>
>>> Rational is to support any I/O library and not fail when the close
>>> is encapsulated.
>>>
>>> Any blocker to swallow this close call?
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau  |  Blog

Re: why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-31 Thread Lukasz Cwik 
I'm not sure what you mean by it closes the door since as the caller of the
library you can create a wrapper filter input stream that ignores close
calls effectively overriding what happens in the UnownedInputStream.

On Wed, Jan 31, 2018 at 10:08 PM, Romain Manni-Bucau 
wrote:

>
>
> Le 1 févr. 2018 03:10, "Lukasz Cwik"  a écrit :
>
> Yes, people will write bad coders which is why this is there. No, I don't
> think swallowing one close is what we want.
>
> In the case where you wants to pass an input/output stream to a library
> that incorrectly assumes ownership, the input/output stream should be
> wrapped right before the call to the library with a filter input/output
> stream that swallows the close and not propagate ignoring this bad behavior
> elsewhere.
>
>
> Hmm,
>
> Elsewhere is nowhere else here since it wouldnt have any side effect
> except not enforcing another layer and making smoothly work for most
> mappers.
>
> Anyway I can live with it but I'm a bit sad it closes the door to the
> easyness to write extensions.
>
>
> On Wed, Jan 31, 2018 at 12:04 PM, Romain Manni-Bucau <
> rmannibu...@gmail.com> wrote:
>
>> Hmm, here we are the ones owning the call since it is in a coder, no? Do
>> we assume people will badly implement coders? In this particular case we
>> can assume close() will be called by a framework I think.
>> What about swallowing one close() and fail on the second?
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau  |  Blog
>>  | Old Blog
>>  | Github
>>  | LinkedIn
>> 
>>
>> 2018-01-31 20:59 GMT+01:00 Lukasz Cwik :
>>
>>> Because people write code like:
>>> myMethod(InputStream in) {
>>>   InputStream child = new InputStream(in);
>>>   child.close();
>>> }
>>>
>>> InputStream in = new FileInputStream(... path ...);
>>> myMethod(in);
>>> myMethod(in);
>>>
>>> An exception will be thrown when the second myMethod call occurs.
>>>
>>> Unfortunately not everyone wraps their calls to a coder with an
>>> UnownedInputStream or a filter input stream which drops close() calls is
>>> why its a problem and in the few places it is done it is used to prevent
>>> bugs from creeping in.
>>>
>>>
>>>
>>> On Tue, Jan 30, 2018 at 11:29 AM, Romain Manni-Bucau <
>>> rmannibu...@gmail.com> wrote:
>>>
 I get the issue but I don't get the last part. Concretely we can
 support any lib by just removing the exception in the close, no? What would
 be the issue? No additional wrapper, no lib integration issue.


 Romain Manni-Bucau
 @rmannibucau  |  Blog
  | Old Blog
  | Github
  | LinkedIn
 

 2018-01-30 19:29 GMT+01:00 Lukasz Cwik :

> Its common in the code base that input and output streams are passed
> around and the caller is responsible for closing it, not the callee. The
> UnownedInputStream is to guard against libraries that are poorly behaved
> and assume they get ownership of the stream when it is given to them.
>
> In the code:
> myMethod(InputStream in) {
>   InputStream child = new InputStream(in);
>   child.close();
> }
>
> InputStream in = ...
> myMethod(in);
> myMethod(in);
> When should "in" be closed?
>
> To get around this issue, create a filter input/output stream that
> ignores close calls like on the JAXB coder:
> https://github.com/apache/beam/blob/master/sdks/java/io/xml/
> src/main/java/org/apache/beam/sdk/io/xml/JAXBCoder.java#L181
>
> We can instead swap around this pattern so that the caller guards
> against callees closing by wrapping with a filter input/output stream but
> this costs an additional method call for each input/output stream call.
>
>
> On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau <
> rmannibu...@gmail.com> wrote:
>
>> Hi guys,
>>
>> All is in the subject ;)
>>
>> Rational is to support any I/O library and not fail when the close is
>> encapsulated.
>>
>> Any blocker to swallow this close call?
>>
>> Romain Manni-Bucau
>> @rmannibucau  |  Blog
>>  | Old Blog
>>  | Github
>>  | LinkedIn
>> 
>>
>
>

>>>
>>
>
>

Re: why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-31 Thread Romain Manni-Bucau 
Le 1 févr. 2018 03:10, "Lukasz Cwik"  a écrit :

Yes, people will write bad coders which is why this is there. No, I don't
think swallowing one close is what we want.

In the case where you wants to pass an input/output stream to a library
that incorrectly assumes ownership, the input/output stream should be
wrapped right before the call to the library with a filter input/output
stream that swallows the close and not propagate ignoring this bad behavior
elsewhere.


Hmm,

Elsewhere is nowhere else here since it wouldnt have any side effect except
not enforcing another layer and making smoothly work for most mappers.

Anyway I can live with it but I'm a bit sad it closes the door to the
easyness to write extensions.


On Wed, Jan 31, 2018 at 12:04 PM, Romain Manni-Bucau 
wrote:

> Hmm, here we are the ones owning the call since it is in a coder, no? Do
> we assume people will badly implement coders? In this particular case we
> can assume close() will be called by a framework I think.
> What about swallowing one close() and fail on the second?
>
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github
>  | LinkedIn
> 
>
> 2018-01-31 20:59 GMT+01:00 Lukasz Cwik :
>
>> Because people write code like:
>> myMethod(InputStream in) {
>>   InputStream child = new InputStream(in);
>>   child.close();
>> }
>>
>> InputStream in = new FileInputStream(... path ...);
>> myMethod(in);
>> myMethod(in);
>>
>> An exception will be thrown when the second myMethod call occurs.
>>
>> Unfortunately not everyone wraps their calls to a coder with an
>> UnownedInputStream or a filter input stream which drops close() calls is
>> why its a problem and in the few places it is done it is used to prevent
>> bugs from creeping in.
>>
>>
>>
>> On Tue, Jan 30, 2018 at 11:29 AM, Romain Manni-Bucau <
>> rmannibu...@gmail.com> wrote:
>>
>>> I get the issue but I don't get the last part. Concretely we can support
>>> any lib by just removing the exception in the close, no? What would be the
>>> issue? No additional wrapper, no lib integration issue.
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau  |  Blog
>>>  | Old Blog
>>>  | Github
>>>  | LinkedIn
>>> 
>>>
>>> 2018-01-30 19:29 GMT+01:00 Lukasz Cwik :
>>>
 Its common in the code base that input and output streams are passed
 around and the caller is responsible for closing it, not the callee. The
 UnownedInputStream is to guard against libraries that are poorly behaved
 and assume they get ownership of the stream when it is given to them.

 In the code:
 myMethod(InputStream in) {
   InputStream child = new InputStream(in);
   child.close();
 }

 InputStream in = ...
 myMethod(in);
 myMethod(in);
 When should "in" be closed?

 To get around this issue, create a filter input/output stream that
 ignores close calls like on the JAXB coder:
 https://github.com/apache/beam/blob/master/sdks/java/io/xml/
 src/main/java/org/apache/beam/sdk/io/xml/JAXBCoder.java#L181

 We can instead swap around this pattern so that the caller guards
 against callees closing by wrapping with a filter input/output stream but
 this costs an additional method call for each input/output stream call.


 On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau <
 rmannibu...@gmail.com> wrote:

> Hi guys,
>
> All is in the subject ;)
>
> Rational is to support any I/O library and not fail when the close is
> encapsulated.
>
> Any blocker to swallow this close call?
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github
>  | LinkedIn
> 
>


>>>
>>
>

Re: why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-31 Thread Romain Manni-Bucau 
Hmm, here we are the ones owning the call since it is in a coder, no? Do we
assume people will badly implement coders? In this particular case we can
assume close() will be called by a framework I think.
What about swallowing one close() and fail on the second?


Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn 

2018-01-31 20:59 GMT+01:00 Lukasz Cwik :

> Because people write code like:
> myMethod(InputStream in) {
>   InputStream child = new InputStream(in);
>   child.close();
> }
>
> InputStream in = new FileInputStream(... path ...);
> myMethod(in);
> myMethod(in);
>
> An exception will be thrown when the second myMethod call occurs.
>
> Unfortunately not everyone wraps their calls to a coder with an
> UnownedInputStream or a filter input stream which drops close() calls is
> why its a problem and in the few places it is done it is used to prevent
> bugs from creeping in.
>
>
>
> On Tue, Jan 30, 2018 at 11:29 AM, Romain Manni-Bucau <
> rmannibu...@gmail.com> wrote:
>
>> I get the issue but I don't get the last part. Concretely we can support
>> any lib by just removing the exception in the close, no? What would be the
>> issue? No additional wrapper, no lib integration issue.
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau  |  Blog
>>  | Old Blog
>>  | Github
>>  | LinkedIn
>> 
>>
>> 2018-01-30 19:29 GMT+01:00 Lukasz Cwik :
>>
>>> Its common in the code base that input and output streams are passed
>>> around and the caller is responsible for closing it, not the callee. The
>>> UnownedInputStream is to guard against libraries that are poorly behaved
>>> and assume they get ownership of the stream when it is given to them.
>>>
>>> In the code:
>>> myMethod(InputStream in) {
>>>   InputStream child = new InputStream(in);
>>>   child.close();
>>> }
>>>
>>> InputStream in = ...
>>> myMethod(in);
>>> myMethod(in);
>>> When should "in" be closed?
>>>
>>> To get around this issue, create a filter input/output stream that
>>> ignores close calls like on the JAXB coder:
>>> https://github.com/apache/beam/blob/master/sdks/java/io/xml/
>>> src/main/java/org/apache/beam/sdk/io/xml/JAXBCoder.java#L181
>>>
>>> We can instead swap around this pattern so that the caller guards
>>> against callees closing by wrapping with a filter input/output stream but
>>> this costs an additional method call for each input/output stream call.
>>>
>>>
>>> On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau <
>>> rmannibu...@gmail.com> wrote:
>>>
 Hi guys,

 All is in the subject ;)

 Rational is to support any I/O library and not fail when the close is
 encapsulated.

 Any blocker to swallow this close call?

 Romain Manni-Bucau
 @rmannibucau  |  Blog
  | Old Blog
  | Github
  | LinkedIn
 

>>>
>>>
>>
>

Re: why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-31 Thread Lukasz Cwik 
Because people write code like:
myMethod(InputStream in) {
  InputStream child = new InputStream(in);
  child.close();
}

InputStream in = new FileInputStream(... path ...);
myMethod(in);
myMethod(in);

An exception will be thrown when the second myMethod call occurs.

Unfortunately not everyone wraps their calls to a coder with an
UnownedInputStream or a filter input stream which drops close() calls is
why its a problem and in the few places it is done it is used to prevent
bugs from creeping in.



On Tue, Jan 30, 2018 at 11:29 AM, Romain Manni-Bucau 
wrote:

> I get the issue but I don't get the last part. Concretely we can support
> any lib by just removing the exception in the close, no? What would be the
> issue? No additional wrapper, no lib integration issue.
>
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github
>  | LinkedIn
> 
>
> 2018-01-30 19:29 GMT+01:00 Lukasz Cwik :
>
>> Its common in the code base that input and output streams are passed
>> around and the caller is responsible for closing it, not the callee. The
>> UnownedInputStream is to guard against libraries that are poorly behaved
>> and assume they get ownership of the stream when it is given to them.
>>
>> In the code:
>> myMethod(InputStream in) {
>>   InputStream child = new InputStream(in);
>>   child.close();
>> }
>>
>> InputStream in = ...
>> myMethod(in);
>> myMethod(in);
>> When should "in" be closed?
>>
>> To get around this issue, create a filter input/output stream that
>> ignores close calls like on the JAXB coder:
>> https://github.com/apache/beam/blob/master/sdks/java/io/xml/
>> src/main/java/org/apache/beam/sdk/io/xml/JAXBCoder.java#L181
>>
>> We can instead swap around this pattern so that the caller guards against
>> callees closing by wrapping with a filter input/output stream but this
>> costs an additional method call for each input/output stream call.
>>
>>
>> On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau <
>> rmannibu...@gmail.com> wrote:
>>
>>> Hi guys,
>>>
>>> All is in the subject ;)
>>>
>>> Rational is to support any I/O library and not fail when the close is
>>> encapsulated.
>>>
>>> Any blocker to swallow this close call?
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau  |  Blog
>>>  | Old Blog
>>>  | Github
>>>  | LinkedIn
>>> 
>>>
>>
>>
>

Re: why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-30 Thread Romain Manni-Bucau 
I get the issue but I don't get the last part. Concretely we can support
any lib by just removing the exception in the close, no? What would be the
issue? No additional wrapper, no lib integration issue.


Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn 

2018-01-30 19:29 GMT+01:00 Lukasz Cwik :

> Its common in the code base that input and output streams are passed
> around and the caller is responsible for closing it, not the callee. The
> UnownedInputStream is to guard against libraries that are poorly behaved
> and assume they get ownership of the stream when it is given to them.
>
> In the code:
> myMethod(InputStream in) {
>   InputStream child = new InputStream(in);
>   child.close();
> }
>
> InputStream in = ...
> myMethod(in);
> myMethod(in);
> When should "in" be closed?
>
> To get around this issue, create a filter input/output stream that ignores
> close calls like on the JAXB coder: https://github.com/apache/
> beam/blob/master/sdks/java/io/xml/src/main/java/org/apache/
> beam/sdk/io/xml/JAXBCoder.java#L181
>
> We can instead swap around this pattern so that the caller guards against
> callees closing by wrapping with a filter input/output stream but this
> costs an additional method call for each input/output stream call.
>
>
> On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau <
> rmannibu...@gmail.com> wrote:
>
>> Hi guys,
>>
>> All is in the subject ;)
>>
>> Rational is to support any I/O library and not fail when the close is
>> encapsulated.
>>
>> Any blocker to swallow this close call?
>>
>> Romain Manni-Bucau
>> @rmannibucau  |  Blog
>>  | Old Blog
>>  | Github
>>  | LinkedIn
>> 
>>
>
>

Re: why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-30 Thread Lukasz Cwik 
Its common in the code base that input and output streams are passed around
and the caller is responsible for closing it, not the callee. The
UnownedInputStream is to guard against libraries that are poorly behaved
and assume they get ownership of the stream when it is given to them.

In the code:
myMethod(InputStream in) {
  InputStream child = new InputStream(in);
  child.close();
}

InputStream in = ...
myMethod(in);
myMethod(in);
When should "in" be closed?

To get around this issue, create a filter input/output stream that ignores
close calls like on the JAXB coder:
https://github.com/apache/beam/blob/master/sdks/java/io/xml/src/main/java/org/apache/beam/sdk/io/xml/JAXBCoder.java#L181

We can instead swap around this pattern so that the caller guards against
callees closing by wrapping with a filter input/output stream but this
costs an additional method call for each input/output stream call.


On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau 
wrote:

> Hi guys,
>
> All is in the subject ;)
>
> Rational is to support any I/O library and not fail when the close is
> encapsulated.
>
> Any blocker to swallow this close call?
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github
>  | LinkedIn
> 
>

why org.apache.beam.sdk.util.UnownedInputStream fails on close instead of ignoring it

2018-01-30 Thread Romain Manni-Bucau 
Hi guys,

All is in the subject ;)

Rational is to support any I/O library and not fail when the close is
encapsulated.

Any blocker to swallow this close call?

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn