Code review comment for lp:~sylvain-pineau/checkbox-core/bug983035

This code will raise an AttributeError when calling the close() method on a None object:

394 + file = create_file(fd, mode)
395 + if not file:
396 + file.close()

By removing the decorators in checkbox.lib.file, the file object no longer contains the errno attribute used like this in checkbox.daemon.pidentry for example:

  elif buffer is None \
       and file.errno != errno.EWOULDBLOCK \
       and file.errno != errno.EAGAIN:
      logging.info("Read failed for pid %d: %d", pipe_fd, file.errno)
      return False

A quick grep returned other places where file.errno is used. We might either want to reintroduce the previous decorators, but that still won't work in places like in checkbox.journal.multi where errno will be called on a None object. How would you address this problem?

By the way, I really like this unit test which clearly shows that the new behavior of fifo_create is much more elegant:

- read_file = File()
- write_file = File()
-
- self.assertTrue(fifo_create(self.path, read_file, write_file))
- self.assertNotEquals(read_file.fileno(), -1)
- self.assertNotEquals(write_file.fileno(), -1)
+ reader, writer = fifo_create(self.path)
+ self.assertNotEquals(reader.fileno(), -1)
+ self.assertNotEquals(writer.fileno(), -1)