RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

[Roger Riggs]

Hi Alan,

Thanks for comments,

On 11/9/2015 6:20 AM, Alan Bateman wrote:
>
>
> On 05/11/2015 21:56, Roger Riggs wrote:
>> Please review the new ProcessBuilder.startPipeline API, 
>> implementation, and tests.
>>
>> :
>>
>> javadoc of ProcessBuilder:  only startPipeline is new:
>>    http://cr.openjdk.java.net/~rriggs/pipedoc/
>>
> I skimmed over the javadoc and it looks quite good.
>
> The method name seems okay, I initially assumed it would be something 
> like startProcesses but that doesn't convey how they are arranged.
>
> Using varargs seem reasonable for the examples like you have in the 
> javadoc but there may be other usages where this might be annoying. 
> Returning List<Process> seems okay. The last Process is of course the 
> most interesting but having some way to get the exitValue of the 
> intermediate processes will be important for logging and 
> troubleshooting purposes.
>
> For testing purposes then I assume the javadoc needs to make it clear 
> that the returned list is the same size as the input array and that 
> the Process at index i corresponds to ProcessBuilder i.
good point.
>
> Did you consider ignoring the redirects of the intermediate processes 
> so that IAE doesn't need to be thrown?
That might be a plus if a ProcessBuilder is being  reused in different 
contexts bot in a out of a pipeline.
But it also might lead to confusing bugs because the internal redirect 
settings are being silently ignored.
It seemed less surprising to flag it as a coding error and throw an 
exception.

>
> I assume the method needs to specify SecurityException and other 
> exceptions that start can throw.
yes,  is it better to specify it as a delegation to the start() method 
or to repeat all of the
specification of start?
>
> On the clean-up then do we need to reassert the interrupt status if 
> interrupted?
yes, fixed.

Thanks, Roger

>
> -Alan.