Arvados

Chapter and verse for the curious.

Reviewing ce12890

• Please update where the command line option help strings refer to /dev/stdout. I think it would be fine to just change them to refer to plain "stdout."
• Under the comment "# Turn on --progress by default", there's a condition args.destination != '/dev/stdout' that no longer works as intended.
• This bug predates your branch, but I noticed it while reading: this block:
  

         if outfile and outfilename != '-':
               os.unlink(outfilename)
  

  
  would better avoid race conditions and other corner cases written as:
  

         if outfile and (outfile.fileno() > 2) and (not outfile.closed):
               os.unlink(outfile.name)
  

  
  (The old code might unlink the wrong file if the ^C comes in between reading a new outfilename and opening it.)

Thanks.

Brett Smith wrote:

Reviewing ce12890

• Please update where the command line option help strings refer to /dev/stdout. I think it would be fine to just change them to refer to plain "stdout."

Fixed.

• Under the comment "# Turn on --progress by default", there's a condition args.destination != '/dev/stdout' that no longer works as intended.

Thanks for catching that, fixed.

• This bug predates your branch, but I noticed it while reading: this block:
  [...]
  would better avoid race conditions and other corner cases written as:
  [...]
  (The old code might unlink the wrong file if the ^C comes in between reading a new outfilename and opening it.)

Done.

Thanks!

Peter Amstutz wrote:

Brett Smith wrote:

• This bug predates your branch, but I noticed it while reading: this block:
  [...]
  would better avoid race conditions and other corner cases written as:
  [...]
  (The old code might unlink the wrong file if the ^C comes in between reading a new outfilename and opening it.)

Done.

Sorry, but, uh, you did not copy and paste this correctly.

• The unlink line should change its argument from outfilename to outfile.name.
• outfile.closed is a boolean attribute, not a method. The condition currently in your branch will raise a TypeError. Remove the parentheses.

The branch is good to merge with these changes. Thanks.