Is there a particular scenario you're worried about here? I can see how it's important to be consistent once in a terminal state, but I don't see how the API makes that worse. Since any of these methods retrieving state ultimately rely on the runner, if the runner is returning different values over time after returning a terminal state, then it's all moot.
To put it another way, let's say I have the following code: pipeline.waitUntilFinish(); State myState = pipeline.getState(); State myState2 = pipeline.getState(); // what guarantee is there that myState == myState2 ? Or let's say I have this: pipeline.cancel(); State myState = pipeline.getState(); // if myState != cancel | fail, you will have angry tests It's noteworthy that waitUntilFinish and cancel result in terminal states, where we shouldn't change to another state after we return from the function. I'm not sure if that is documented/the consensus of the community, but it's been how I think about those states. If a runner were changing states after being in a terminal state, then that can cause issues. People like to write their tests/production code using pipeline.waitUntilFinish() and then retrieving state, look at results, etc... immediately after that. However, the fact that waitUntilFinish returns a state doesn't cause that problem - it's that the runner impl returns different values over time and that's true even if we only return state from getState. Is there a particular scenario driving your question? S On Thu, Dec 22, 2016 at 10:29 AM Stas Levin <[email protected]> wrote: > I see, thanks for the snippet. > > Won't the API be more robust (i.e. not leave it up to implementors' > interpretation) by not exposing the state in any way other than getState()? > The snippet above will still work by mutating existing state, and in > addition such an API will prevent returning any other (possibly > inconsistent) state. > > So instead of doing: > > State myState = pipeline.waitUntilFinish(); > State myOtherState = pipeline.getState(); > > // technically (myState != state) can be true > > Users will do: > > pipeline.waitUntilFinish(); > State myState = pipeline.getState(); > > What do you think? > > > On Thu, Dec 22, 2016 at 7:32 PM Lukasz Cwik <[email protected]> > wrote: > > > I think cancel and waitUntilFinish return state because they are > > interacting with the runner and are likely to have pipeline state > > information available to them at the time when performing that operation. > > > > For example, waitUntilFinish(Duration) could just be a thin veneer of: > > State state; > > do { > > state = getState(); > > if (state is terminal) { > > return state; > > } > > sleep > > } while (time remaining) > > return state; > > > > > > > > > > On Thu, Dec 22, 2016 at 3:18 AM, Stas Levin <[email protected]> wrote: > > > > > Hi all, > > > > > > I was wondering if the current API for PipelineResult might open the > door > > > to inconsistencies stemming from cancel() or waitUntilFinish() > returning > > > one state, and getState() returning another? Are such cases legit? > > > > > > PipelineResult's API has a getState() method: > > > > > > State getState(); > > > > > > at the same time other methods such as cancel() and waitUntilFinish() > > > return State as well: > > > > > > State waitUntilFinish(Duration duration); > > > > > > State cancel() throws IOException; > > > > > > Is this intentional? > > > > > > The alternative would be making cancel() and waitUntilFinish() return > > void, > > > so that the only (and thus consistent) way to obtain a PipelineResult's > > > state would be via getState(). > > > > > > Am I missing something? > > > > > > Regards, > > > Stas > > > > > >
