That is right and even if all the IFrameWriter instances share the same inner writer, the UnionOperator make sure the contract is maintained with that writer. I see it clearly now.
Thanks, Abdullah. Amoudi, Abdullah. On Thu, Dec 10, 2015 at 8:32 AM, Yingyi Bu <[email protected]> wrote: > There are multiple IFrameWriters created by the IOperatorNodePushable, one > for each input. > It doesn't violate the contract. > > Best, > Yingyi > > On Thu, Dec 10, 2015 at 7:55 AM, abdullah alamoudi <[email protected]> > wrote: > > > However, > > Have a look at UnionAllOperatorDescriptor. For the IFrameWriter created > > here, it needs to accept multiple open calls and multiple close calls > since > > it has multiple inputs and a single output. > > This doesn't conform to the allowed calls as per the contract specified > in > > the document. > > > > Cheers, > > Abdullah. > > > > Amoudi, Abdullah. > > > > On Wed, Dec 9, 2015 at 11:52 AM, abdullah alamoudi <[email protected]> > > wrote: > > > > > I think you're right. > > > > > > Cheers, > > > Abdullah. > > > > > > Amoudi, Abdullah. > > > > > > On Wed, Dec 9, 2015 at 11:48 AM, Till Westmann <[email protected]> > wrote: > > > > > >> I think that the state diagram says that it’s not allowed. > > >> However, I don’t think that each implementation should check that, as > > >> that would require additional state and code in each operator that > > should > > >> not be needed (if everybody sticks to the contract). > > >> If we want to check that the contract is maintained we should do that > > >> either in the unit tests or by introducing a wrapper that checks the > > >> contract on top of the operator itself. This wrapper could then be > added > > >> during plan generation if indicated by a user (e.g. by setting a > flag). > > >> > > >> My 2c, > > >> Till > > >> > > >> On 9 Dec 2015, at 10:56, abdullah alamoudi wrote: > > >> > > >> There seems to be something missing here which is the legal function > > calls > > >>> that can be made in each state. For example: > > >>> > > >>> 1. Is it legal to call open() on an open instance of IFrameWriter? If > > >>> not, > > >>> then is it the responsibility of the IFrameWriter to throw an > exception > > >>> in > > >>> that case > > >>> ? > > >>> 2. Is it legal to call close() on a closed instance of IFrameWriter? > If > > >>> not, then again, who's responsibility is it to throw an exception in > > that > > >>> case? > > >>> 3. Is it legal to call fail() on a failed instance of IFrameWriter? > If > > >>> not, > > >>> then again, who's responsibility is it to throw an exception in that > > >>> case? > > >>> > > >>> I am asking this because I think that is happening in some places in > > the > > >>> code already. > > >>> Should we allow that? > > >>> > > >>> Abdullah. > > >>> > > >>> Amoudi, Abdullah. > > >>> > > >>> On Tue, Dec 8, 2015 at 11:56 PM, Mike Carey <[email protected]> > wrote: > > >>> > > >>> +1 > > >>>> > > >>>> On 12/8/15 5:43 PM, Till Westmann wrote: > > >>>> > > >>>> Hi, > > >>>>> > > >>>>> When improving test coverage we noticed, that there are some > > >>>>> inconsistencies wrt. state management and transitions in different > > >>>>> implementations of IFramewriter. These inconsistencies cause a > number > > >>>>> of > > >>>>> test failures - especially when testing error conditions. Ian has > > >>>>> written > > >>>>> up a document [1] on the current contract (which is unfortunately > not > > >>>>> consistently implemented) and on an alternate contract that we have > > >>>>> discussed. > > >>>>> We currently believe that the alternate contract would be > preferable > > as > > >>>>> there is more similarity in the approach to handling errors during > > >>>>> open() > > >>>>> or nextFrame(). > > >>>>> Our proposal is to > > >>>>> 1) Adopt the alternate contract and document it in > > >>>>> a) the IFramewriter javadoc and > > >>>>> b) a design document. > > >>>>> 2) Implement new operators according to the alternate contract. > > >>>>> 3) Add mock-based unit tests (e.g. using Mockito [2]) for new > > operators > > >>>>> that verify that the contract is maintained. > > >>>>> 4) Add mock-based using test for existing operators as bugs are > found > > >>>>> and > > >>>>> fixed. > > >>>>> > > >>>>> Please take a look at the document and tell us, what you think > about > > >>>>> the > > >>>>> contract and the approach. > > >>>>> > > >>>>> Thanks, > > >>>>> Abdullah, Ian, Yingyi, and Till > > >>>>> > > >>>>> [1] > > >>>>> > > >>>>> > > > https://docs.google.com/document/d/1QJ8X7QFMMax5D5k9RVje6hoNr_SNZSW9Oshzv7DpcQw/edit?usp=sharing > > >>>>> [2] http://mockito.org/ > > >>>>> > > >>>>> > > >>>> > > >>>> > > > > > >
