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" <lc...@google.com> 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" <lc...@google.com> 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 <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> >>> >>> 2018-01-31 20:59 GMT+01:00 Lukasz Cwik <lc...@google.com>: >>> >>>> 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 <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> >>>>> >>>>> 2018-01-30 19:29 GMT+01:00 Lukasz Cwik <lc...@google.com>: >>>>> >>>>>> 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 <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> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> >> >
