checkbox-core

Merge lp:~sylvain-pineau/checkbox-core/bug983035 into lp:checkbox-core

  1. bug983035
  2. Merge into trunk
Proposed by Sylvain Pineau
Status: Needs review
Proposed branch: lp:~sylvain-pineau/checkbox-core/bug983035
Merge into: lp:checkbox-core
Diff against target: 775 lines (+147/-184)
16 files modified
checkbox/daemon/pidentry.py (+3/-3)
checkbox/daemon/server.py (+8/-3)
checkbox/io/fifo.py (+5/-4)
checkbox/io/pipe.py (+7/-5)
checkbox/io/watchdog.py (+4/-4)
checkbox/journal/multi.py (+3/-3)
checkbox/journal/reader.py (+2/-2)
checkbox/journal/tests/test_reader.py (+2/-2)
checkbox/journal/tests/test_writer.py (+2/-2)
checkbox/journal/writer.py (+21/-22)
checkbox/lib/fifo.py (+25/-10)
checkbox/lib/file.py (+19/-77)
checkbox/lib/tests/test_fifo.py (+3/-7)
checkbox/lib/tests/test_file.py (+38/-35)
checkbox/runner/manager.py (+3/-3)
checkbox/runner/starter.py (+2/-2)
To merge this branch: bzr merge lp:~sylvain-pineau/checkbox-core/bug983035
Reviewer Review Type Date Requested Status
Marc Tardif Needs Fixing
Sylvain Pineau (community) Needs Resubmitting
Review via email: mp+103270@code.launchpad.net

Description of the change

This MR replaces the file wrapper class with a new one that inherits of io.fileIO.
Classes depending on the old file interface have been updated as well.

To post a comment you must log in.
Revision history for this message
Marc Tardif (cr3) wrote :

Excellent work looking info fileobject.c! I just commented on the bug linked to this branch a couple minutes ago, do you think we should add more methods to this story?

review: Needs Information
Reply
lp:~sylvain-pineau/checkbox-core/bug983035 updated
15. By Marc Tardif

Merged from javier.collado's staticmethods branch.

16. By Marc Tardif

Preliminary submit and shadowd components.

17. By Marc Tardif

Merged from javier.collado's package-docstrings branch.

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

A new proposal based this time on io.FileIO

review: Needs Resubmitting
Reply
lp:~sylvain-pineau/checkbox-core/bug983035 updated
18. By Sylvain Pineau

merge trunk recent updates

19. By Sylvain Pineau

File class now inherits from io.FileIO

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

io.FileIO is now used as a base class to checkbox.lib.file

review: Needs Resubmitting
Reply
lp:~sylvain-pineau/checkbox-core/bug983035 updated
20. By Sylvain Pineau

Fix classes that now depends on the new File/io.FileIO class

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

I've fixed classes that used the old file interface to match the new way we built file objects.

review: Needs Resubmitting
Reply
lp:~sylvain-pineau/checkbox-core/bug983035 updated
21. By Sylvain Pineau

fix reamining calls to File.create_from*

22. By Sylvain Pineau

Remove unused import of File in watchdog.py

Revision history for this message
Marc Tardif (cr3) wrote :

Nice work, I'm very pleased about removing the create_from_* class methods and using None instead of an empty File instance! First, I don't think this logic is the same as before:

324 - if (self._user_enable
325 - and not self._openPath(path, self._file, lock=self._lock)):
326 + self._file = self._openPath(path, lock=self._lock)
327 + if (self._user_enable and not self._file):

Before, _openPath was only called if _user_enable is True. Now, _openPath is always called. Second, I think this code is a bit ambiguous because self._writer is not expected to be boolean:

141 + return self._writer == True

Even though your code probably works, I think it's more obvious to say something like: return self._writer is not None. Last, but not least, what would you think about changing Fifo.create_fifo to just be a function, instead of a class method, that would return a reader and writer tuple?

review: Needs Fixing
Reply
lp:~sylvain-pineau/checkbox-core/bug983035 updated
23. By Sylvain Pineau

Fix a call to _openPath in writer.py and rewrite the returned boolean of connect and listen functions in pipe.py

24. By Sylvain Pineau

Revert Fifo.create_fifo into a function instead of a class method returning a reader and writer tuple

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Thanks for the review, I've made the suggested changes, both in code and tests.

review: Needs Resubmitting
Reply
Revision history for this message
Marc Tardif (cr3) wrote :

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)

review: Needs Fixing
Reply
Revision history for this message
Marc Tardif (cr3) wrote :

I just thought of a solution to solve the problem of having create_file somehow return an errno: instead of returning None when an error occurs, we could return the negative errno:

  file = create_file(path)
  if file < 0:
    logging.error("Failed to open file '%s'; errno = %d", path, -file)

This will work when create_file returns an actual object, so the code is safe. The only problem is that the value assigned to file is either an object or a negative integer. This might sound strange but it's not that much diffferent from returning None. For example, both a negative integer and None will raise an AttributeError when attempting to call file.read() when create_file fails. What do you think?

Reply
Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

It's probably the best we can do to keep the calling code quite simple.

I was thinking to reintroduce the errno attribute to the File class, if the file object creation fails, returns the class and check for the errno. Given that the application if not threaded, it could be reliable:

class File(io.FileIO):

    __slots__ = (
        "errno",
        )

    def __init__(self, *args, **kwargs):
        super(File, self).__init__(*args, **kwargs)
        self.errno = 0

[...]

def create_file(*args, **kwargs):
    try:
        return File(*args, **kwargs)
    except (EnvironmentError, ValueError), error:
        f = File
        f.errno = error.errno
        return f

Even if I prefer your proposal, what do you think of mine ?

Reply

Unmerged revisions

24. By Sylvain Pineau

Revert Fifo.create_fifo into a function instead of a class method returning a reader and writer tuple

23. By Sylvain Pineau

Fix a call to _openPath in writer.py and rewrite the returned boolean of connect and listen functions in pipe.py

22. By Sylvain Pineau

Remove unused import of File in watchdog.py

21. By Sylvain Pineau

fix reamining calls to File.create_from*

20. By Sylvain Pineau

Fix classes that now depends on the new File/io.FileIO class

19. By Sylvain Pineau

File class now inherits from io.FileIO

18. By Sylvain Pineau

merge trunk recent updates

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'checkbox/daemon/pidentry.py'
--- checkbox/daemon/pidentry.py 2012-04-23 19:36:19 +0000
+++ checkbox/daemon/pidentry.py 2012-05-02 10:18:20 +0000
@@ -26,7 +26,7 @@
26import logging26import logging
2727
28from checkbox.lib.file import (28from checkbox.lib.file import (
29 File,29 create_file,
30 fd_close,30 fd_close,
31 )31 )
3232
@@ -78,7 +78,7 @@
78 max_read_bytes = (78 max_read_bytes = (
79 self.max_pipe_buffer - len(self.pipe_buffer[pipe_index]))79 self.max_pipe_buffer - len(self.pipe_buffer[pipe_index]))
8080
81 file = File.create_from_fd(pipe_fd, "r")81 file = create_file(pipe_fd, "r")
82 buffer = file.read(max_read_bytes)82 buffer = file.read(max_read_bytes)
83 if buffer:83 if buffer:
84 self.pipe_buffer[pipe_index] += buffer84 self.pipe_buffer[pipe_index] += buffer
@@ -100,7 +100,7 @@
100 data_left = buffer[self.stdin_offset:]100 data_left = buffer[self.stdin_offset:]
101 total_len = len(buffer)101 total_len = len(buffer)
102102
103 file = File.create_from_fd(pipe_fd, "w")103 file = create_file(pipe_fd, "w")
104 bytes_written = file.write(data_left)104 bytes_written = file.write(data_left)
105105
106 else:106 else:
107107
=== modified file 'checkbox/daemon/server.py'
--- checkbox/daemon/server.py 2012-04-23 19:36:19 +0000
+++ checkbox/daemon/server.py 2012-05-02 10:18:20 +0000
@@ -52,7 +52,12 @@
52from time import time52from time import time
5353
54from checkbox.lib.enum import Enum54from checkbox.lib.enum import Enum
55from checkbox.lib.file import File, fd_close, fd_get_flags, fd_set_flags55from checkbox.lib.file import (
56 create_file,
57 fd_close,
58 fd_get_flags,
59 fd_set_flags,
60 )
56from checkbox.lib.sigset import Sigset61from checkbox.lib.sigset import Sigset
57from checkbox.lib.stat import Stat62from checkbox.lib.stat import Stat
5863
@@ -653,14 +658,14 @@
653 if size < 0:658 if size < 0:
654 raise Exception("Invalid size: %d" % size)659 raise Exception("Invalid size: %d" % size)
655660
656 pipe_file = File.create_from_fd(pipe_end, "r")661 pipe_file = create_file(pipe_end, "r")
657 return pipe_file.read(size)662 return pipe_file.read(size)
658663
659 def writePipe(self, pipe_end, buffer):664 def writePipe(self, pipe_end, buffer):
660 if not buffer:665 if not buffer:
661 raise Exception("Invalid buffer, empty")666 raise Exception("Invalid buffer, empty")
662667
663 pipe_file = File.create_from_fd(pipe_end, "w")668 pipe_file = create_file(pipe_end, "w")
664 return pipe_file.write(buffer)669 return pipe_file.write(buffer)
665670
666 def closeFd(self, fd):671 def closeFd(self, fd):
667672
=== modified file 'checkbox/io/fifo.py'
--- checkbox/io/fifo.py 2012-04-24 22:43:09 +0000
+++ checkbox/io/fifo.py 2012-05-02 10:18:20 +0000
@@ -33,7 +33,7 @@
33 fifo_create,33 fifo_create,
34 )34 )
35from checkbox.lib.file import (35from checkbox.lib.file import (
36 File,36 create_file,
37 fd_open,37 fd_open,
38 fd_get_flags,38 fd_get_flags,
39 fd_set_flags39 fd_set_flags
@@ -78,7 +78,7 @@
78 self._pid = 078 self._pid = 0
79 self._serial_number = -179 self._serial_number = -1
8080
81 self._dummy = File()81 self._dummy = None
82 self._state = FifoState.VIRGIN82 self._state = FifoState.VIRGIN
8383
84 def __del__(self):84 def __del__(self):
@@ -267,7 +267,8 @@
267267
268 def _openReader(self, path):268 def _openReader(self, path):
269 # Keep dummy writer open so that reader never receives EOF269 # Keep dummy writer open so that reader never receives EOF
270 if not fifo_create(path, self._reader, self._dummy):270 self._reader, self._dummy = fifo_create(path)
271 if not self._reader or not self._dummy:
271 logging.info("Failed to set fifo path at %s", path)272 logging.info("Failed to set fifo path at %s", path)
272 return False273 return False
273274
@@ -289,7 +290,7 @@
289 if fd_set_flags(write_fd, flags) < 0:290 if fd_set_flags(write_fd, flags) < 0:
290 return False291 return False
291292
292 self._writer.initialize_from_fd(write_fd, "w")293 self._writer = create_file(write_fd, "w")
293294
294 return True295 return True
295296
296297
=== modified file 'checkbox/io/pipe.py'
--- checkbox/io/pipe.py 2012-04-23 19:36:19 +0000
+++ checkbox/io/pipe.py 2012-05-02 10:18:20 +0000
@@ -24,7 +24,7 @@
2424
25import logging25import logging
2626
27from checkbox.lib.file import File27from checkbox.lib.file import create_file
2828
29from checkbox.io.selector import (29from checkbox.io.selector import (
30 Selector,30 Selector,
@@ -54,8 +54,8 @@
54 super(Pipe, self).__init__()54 super(Pipe, self).__init__()
55 self._fd = None55 self._fd = None
56 self._bytes = ""56 self._bytes = ""
57 self._reader = File()57 self._reader = None
58 self._writer = File()58 self._writer = None
59 self._watchdog_client = None59 self._watchdog_client = None
60 self._watchdog_server = None60 self._watchdog_server = None
6161
@@ -71,11 +71,13 @@
7171
72 def connect(self, fd):72 def connect(self, fd):
73 self._fd = fd73 self._fd = fd
74 return self._writer.initialize_from_fd(fd, "w") == 074 self._writer = create_file(fd, "w")
75 return self._writer is not None
7576
76 def listen(self, fd):77 def listen(self, fd):
77 self._fd = fd78 self._fd = fd
78 return self._reader.initialize_from_fd(fd, "r") == 079 self._reader = create_file(fd, "r")
80 return self._reader is not None
7981
80 def handleIncomingMessage(self):82 def handleIncomingMessage(self):
81 """Receive messages from the reader and buffer them.83 """Receive messages from the reader and buffer them.
8284
=== modified file 'checkbox/io/watchdog.py'
--- checkbox/io/watchdog.py 2012-04-23 19:36:19 +0000
+++ checkbox/io/watchdog.py 2012-05-02 10:18:20 +0000
@@ -28,7 +28,6 @@
2828
29from checkbox.lib.fifo import fifo_create29from checkbox.lib.fifo import fifo_create
30from checkbox.lib.file import (30from checkbox.lib.file import (
31 File,
32 fd_open,31 fd_open,
33 fd_close,32 fd_close,
34 )33 )
@@ -88,8 +87,8 @@
8887
89 def __init__(self):88 def __init__(self):
90 self._path = None89 self._path = None
91 self._reader = File()90 self._reader = None
92 self._writer = File()91 self._writer = None
9392
94 def __del__(self):93 def __del__(self):
95 self.reset()94 self.reset()
@@ -108,7 +107,8 @@
108 return False107 return False
109108
110 path += WATCHDOG_SUFFIX109 path += WATCHDOG_SUFFIX
111 if not fifo_create(path, self._reader, self._writer):110 self._reader, self._writer = fifo_create(path)
111 if not self._reader or not self._writer:
112 logging.info("Failed to set watched fifo path at %s", path)112 logging.info("Failed to set watched fifo path at %s", path)
113 return False113 return False
114114
115115
=== modified file 'checkbox/journal/multi.py'
--- checkbox/journal/multi.py 2012-04-24 13:23:07 +0000
+++ checkbox/journal/multi.py 2012-05-02 10:18:20 +0000
@@ -30,7 +30,7 @@
3030
31from checkbox.lib.directory import Directory31from checkbox.lib.directory import Directory
32from checkbox.lib.file import (32from checkbox.lib.file import (
33 File,33 create_file,
34 fd_open,34 fd_open,
35 fd_close,35 fd_close,
36 )36 )
@@ -74,7 +74,7 @@
7474
75 :param filename: Filename to read.75 :param filename: Filename to read.
76 """76 """
77 file = File.create_from_path(filename, "r")77 file = create_file(filename, "r")
78 if not file:78 if not file:
79 logging.debug(79 logging.debug(
80 "Failed to open '%s' with errno %d", filename, file.errno)80 "Failed to open '%s' with errno %d", filename, file.errno)
@@ -184,7 +184,7 @@
184 if directory:184 if directory:
185 submit_filename = os.path.join(directory, submit_filename)185 submit_filename = os.path.join(directory, submit_filename)
186186
187 file = File.create_from_path(submit_filename, "r")187 file = create_file(submit_filename, "r")
188 if not file:188 if not file:
189 logging.debug(189 logging.debug(
190 "Failed to open '%s' with errno %d",190 "Failed to open '%s' with errno %d",
191191
=== modified file 'checkbox/journal/reader.py'
--- checkbox/journal/reader.py 2012-04-23 19:36:19 +0000
+++ checkbox/journal/reader.py 2012-05-02 10:18:20 +0000
@@ -31,7 +31,7 @@
31from checkbox.lib import param31from checkbox.lib import param
32from checkbox.lib.enum import Enum32from checkbox.lib.enum import Enum
33from checkbox.lib.file import (33from checkbox.lib.file import (
34 File,34 create_file,
35 fd_open,35 fd_open,
36 fd_close,36 fd_close,
37 )37 )
@@ -644,7 +644,7 @@
644 return EventOutcome.READ_ERROR644 return EventOutcome.READ_ERROR
645645
646 try:646 try:
647 self._file = File.create_from_fd(self._fd, "r")647 self._file = create_file(self._fd, "r")
648 except OSError:648 except OSError:
649 self._closeJournalFile(True)649 self._closeJournalFile(True)
650 return EventOutcome.READ_ERROR650 return EventOutcome.READ_ERROR
651651
=== modified file 'checkbox/journal/tests/test_reader.py'
--- checkbox/journal/tests/test_reader.py 2012-04-23 19:36:19 +0000
+++ checkbox/journal/tests/test_reader.py 2012-05-02 10:18:20 +0000
@@ -26,7 +26,7 @@
2626
27from tempfile import mkstemp27from tempfile import mkstemp
2828
29from checkbox.lib.file import File29from checkbox.lib.file import create_file
3030
31from checkbox.journal.event import (31from checkbox.journal.event import (
32 Event,32 Event,
@@ -45,7 +45,7 @@
4545
46 def setUp(self):46 def setUp(self):
47 (fd, self.path) = mkstemp()47 (fd, self.path) = mkstemp()
48 self.file = File.create_from_fd(fd)48 self.file = create_file(fd)
4949
50 os.environ["EVENT_JOURNAL"] = self.path50 os.environ["EVENT_JOURNAL"] = self.path
5151
5252
=== modified file 'checkbox/journal/tests/test_writer.py'
--- checkbox/journal/tests/test_writer.py 2012-04-23 19:36:19 +0000
+++ checkbox/journal/tests/test_writer.py 2012-05-02 10:18:20 +0000
@@ -27,7 +27,7 @@
27from tempfile import mkstemp27from tempfile import mkstemp
28from StringIO import StringIO28from StringIO import StringIO
2929
30from checkbox.lib.file import File30from checkbox.lib.file import create_file
3131
32from checkbox.journal.event import (32from checkbox.journal.event import (
33 Event,33 Event,
@@ -40,7 +40,7 @@
4040
41 def setUp(self):41 def setUp(self):
42 (fd, self.path) = mkstemp()42 (fd, self.path) = mkstemp()
43 self.file = File.create_from_fd(fd)43 self.file = create_file(fd)
4444
45 os.environ["EVENT_JOURNAL"] = self.path45 os.environ["EVENT_JOURNAL"] = self.path
4646
4747
=== modified file 'checkbox/journal/writer.py'
--- checkbox/journal/writer.py 2012-04-23 19:36:19 +0000
+++ checkbox/journal/writer.py 2012-05-02 10:18:20 +0000
@@ -34,7 +34,7 @@
34 )34 )
35from checkbox.lib.file import (35from checkbox.lib.file import (
36 FILE_NULL,36 FILE_NULL,
37 File,37 create_file,
38 fd_open,38 fd_open,
39 fd_close,39 fd_close,
40 )40 )
@@ -96,13 +96,13 @@
9696
97 self._user_enable = True97 self._user_enable = True
98 self._path = ""98 self._path = ""
99 self._file = File()99 self._file = None
100 self._lock = Lock()100 self._lock = Lock()
101 self._enable_fsync = True101 self._enable_fsync = True
102 self._enable_locking = True102 self._enable_locking = True
103103
104 self._global_path = ""104 self._global_path = ""
105 self._global_file = File()105 self._global_file = None
106 self._global_lock = Lock()106 self._global_lock = Lock()
107 self._global_stat = None107 self._global_stat = None
108108
@@ -186,12 +186,13 @@
186 self._path = path186 self._path = path
187187
188 # Reset local resources188 # Reset local resources
189 self._file.reset()189 self._file = None
190 self._lock.reset()190 self._lock.reset()
191191
192 if (self._user_enable192 if self._user_enable:
193 and not self._openPath(path, self._file, lock=self._lock)):193 self._file = self._openPath(path, lock=self._lock)
194 return False194 if not self._file:
195 return False
195196
196 return self.initialize()197 return self.initialize()
197198
@@ -200,13 +201,14 @@
200 self._global_lock.reset()201 self._global_lock.reset()
201202
202 if self._global_file:203 if self._global_file:
203 self._global_file.reset()204 self._global_file = None
204205
205 if not self._global_path:206 if not self._global_path:
206 return True207 return True
207208
208 if not self._openPath(209 self._global_file = self._openPath(
209 self._global_path, self._global_file, lock=self._global_lock):210 self._global_path, lock=self._global_lock)
211 if not self._global_file:
210 return False212 return False
211213
212 stat = Stat()214 stat = Stat()
@@ -290,7 +292,7 @@
290 # Rotate the journal292 # Rotate the journal
291293
292 # Read the old header, use it to write an updated one294 # Read the old header, use it to write an updated one
293 file = File.create_from_path(self._global_path, "r")295 file = create_file(self._global_path, "r")
294 if not file:296 if not file:
295 logging.debug("Failed to open '%s'", self._global_path)297 logging.debug("Failed to open '%s'", self._global_path)
296298
@@ -311,12 +313,11 @@
311313
312 header_reader.num_events = events314 header_reader.num_events = events
313315
314 file.reset()
315
316 header_reader.size = current_filesize316 header_reader.size = current_filesize
317317
318 # Craft a header writer object from the header reader318 # Craft a header writer object from the header reader
319 if not self._openPath(self._global_path, file, False):319 file = self._openPath(self._global_path, False)
320 if not file:
320 logging.debug(321 logging.debug(
321 "Failed to open '%s' for header rewrite", self._global_path)322 "Failed to open '%s' for header rewrite", self._global_path)
322323
@@ -381,7 +382,7 @@
381 return True382 return True
382383
383 # Reset global resources384 # Reset global resources
384 self._global_file.reset()385 self._global_file = None
385 self._global_lock.reset()386 self._global_lock.reset()
386 self._global_stat = None387 self._global_stat = None
387388
@@ -430,9 +431,8 @@
430431
431 return True432 return True
432433
433 def _openPath(self, path, file, append=True, lock=None):434 def _openPath(self, path, append=True, lock=None):
434 if path and path == FILE_NULL:435 if path and path == FILE_NULL:
435 file.reset()
436 if lock is not None:436 if lock is not None:
437 lock.reset()437 lock.reset()
438 return True438 return True
@@ -446,16 +446,15 @@
446 return False446 return False
447447
448 mode = "a" if append else "w"448 mode = "a" if append else "w"
449 try:449 file = create_file(fd, mode)
450 file.initialize_from_fd(fd, mode)450 if not file:
451 except OSError:451 file.close()
452 fd_close(fd)
453 return False452 return False
454453
455 if lock is not None:454 if lock is not None:
456 lock.initialize(fd, file)455 lock.initialize(fd, file)
457456
458 return True457 return file
459458
460 def writeEvent(self, event):459 def writeEvent(self, event):
461 """Write an event to the journal file.460 """Write an event to the journal file.
462461
=== modified file 'checkbox/lib/fifo.py'
--- checkbox/lib/fifo.py 2012-04-23 19:36:19 +0000
+++ checkbox/lib/fifo.py 2012-05-02 10:18:20 +0000
@@ -30,13 +30,18 @@
30from checkbox.lib.file import (30from checkbox.lib.file import (
31 fd_open,31 fd_open,
32 fd_close,32 fd_close,
33 create_file,
33 )34 )
3435
3536
36FIFO_BUF = 51237FIFO_BUF = 512
3738
3839
39def fifo_create(path, reader, writer):40def fifo_create(path):
41 reader = None
42 writer = None
43 uninitialized_fh = reader, writer
44
40 # Check that fifo doesn't exist45 # Check that fifo doesn't exist
41 if os.path.exists(path):46 if os.path.exists(path):
42 os.unlink(path)47 os.unlink(path)
@@ -46,13 +51,18 @@
46 os.mkfifo(path, 0600)51 os.mkfifo(path, 0600)
47 except OSError, error:52 except OSError, error:
48 logging.info("Failed to create fifo %s: %d", path, error.errno)53 logging.info("Failed to create fifo %s: %d", path, error.errno)
49 return False54 return uninitialized_fh
5055
51 # Open read side of the fifo initially in non-blocking mode56 # Open read side of the fifo initially in non-blocking mode
52 read_fd = fd_open(path, os.O_RDONLY | os.O_NONBLOCK)57 read_fd = fd_open(path, os.O_RDONLY | os.O_NONBLOCK)
53 if read_fd == -1:58 if read_fd == -1:
54 logging.info("Failed to open read-only %s", path)59 logging.info("Failed to open read-only %s", path)
55 return False60 return uninitialized_fh
61 reader = create_file(read_fd, "r")
62 if not reader:
63 logging.info("Failed to initialize file descriptors.")
64 fd_close(read_fd)
65 return uninitialized_fh
5666
57 # Set the fifo back to blocking mode67 # Set the fifo back to blocking mode
58 flags = fcntl.fcntl(read_fd, fcntl.F_GETFL)68 flags = fcntl.fcntl(read_fd, fcntl.F_GETFL)
@@ -63,14 +73,19 @@
63 if write_fd == -1:73 if write_fd == -1:
64 logging.info("Failed to open write-only %s", path)74 logging.info("Failed to open write-only %s", path)
65 fd_close(read_fd)75 fd_close(read_fd)
66 return False76 return uninitialized_fh
6777 writer = create_file(write_fd, "w")
68 if (reader.initialize_from_fd(read_fd, "r") or78 if not writer:
69 writer.initialize_from_fd(write_fd, "w")):
70 logging.info("Failed to initialize file descriptors.")79 logging.info("Failed to initialize file descriptors.")
71 fd_close(write_fd)80 fd_close(write_fd)
72 fd_close(read_fd)81 reader.close()
73 return False82 return uninitialized_fh
83
84 if (not reader.readable() or not writer.writable()):
85 logging.info("Failed to initialize file descriptors.")
86 writer.close()
87 reader.close()
88 return uninitialized_fh
7489
75 # Everything worked90 # Everything worked
76 return True91 return reader, writer
7792
=== modified file 'checkbox/lib/file.py'
--- checkbox/lib/file.py 2012-04-23 19:36:19 +0000
+++ checkbox/lib/file.py 2012-05-02 10:18:20 +0000
@@ -21,6 +21,7 @@
21__all__ = [21__all__ = [
22 "FILE_NULL",22 "FILE_NULL",
23 "File",23 "File",
24 "create_file",
24 "fd_open",25 "fd_open",
25 "fd_close",26 "fd_close",
26 "fd_get_flags",27 "fd_get_flags",
@@ -28,92 +29,33 @@
28 ]29 ]
2930
30import os31import os
32import io
31import fcntl33import fcntl
3234
33from checkbox.lib.decorators import CheckException
34
3535
36FILE_NULL = "/dev/null"36FILE_NULL = "/dev/null"
3737
3838
39class CheckError(CheckException):39class File(io.FileIO):
4040
41 def onError(self, error):
42 self._instance.errno = error.errno
43
44
45def check_error(default=None):
46
47 def wrapper(func):
48 return CheckError(func, default, EnvironmentError)
49
50 return wrapper
51
52
53class File:
54
55 __slots__ = (
56 "info",
57 "errno",
58 )
59
60 def __init__(self):
61 self.info = None
62 self.errno = 0
63
64 @classmethod
65 def create_from_path(cls, *args, **kwargs):
66 file = cls()
67 file.initialize_from_path(*args, **kwargs)
68
69 return file
70
71 @classmethod
72 def create_from_fd(cls, *args, **kwargs):
73 file = cls()
74 file.initialize_from_fd(*args, **kwargs)
75
76 return file
77
78 @check_error(-1)
79 def initialize_from_path(self, *args, **kwargs):
80 self.info = open(*args, **kwargs)
81 return 0
82
83 @check_error(-1)
84 def initialize_from_fd(self, *args, **kwargs):
85 self.info = os.fdopen(*args, **kwargs)
86 return 0
87
88 def copy_from(self, other):
89 self.info = other.info
90
91 def reset(self):
92 self.info = None
93 self.errno = 0
94
95 @check_error(None)
96 def read(self, size):41 def read(self, size):
97 # Return something useful42 try:
98 return os.read(self.info.fileno(), size)43 return super(File, self).read(size)
44 except (EnvironmentError, ValueError):
45 return None
9946
100 @check_error(-1)
101 def write(self, buffer):47 def write(self, buffer):
102 # Return something useful48 try:
103 return os.write(self.info.fileno(), buffer)49 return super(File, self).write(buffer)
10450 except (EnvironmentError, ValueError):
105 @check_error(None)51 return -1
106 def __getattr__(self, name):52
107 return getattr(self.info, name)53
10854def create_file(*args, **kwargs):
109 def __setattr__(self, name, value):55 try:
110 if name in File.__slots__:56 return File(*args, **kwargs)
111 return super(File, self).__setattr__(name, value)57 except (EnvironmentError, ValueError):
11258 return None
113 return setattr(self.info, name, value)
114
115 def __nonzero__(self):
116 return self.info is not None
11759
11860
119def fd_open(path, flag, *args, **kwargs):61def fd_open(path, flag, *args, **kwargs):
12062
=== modified file 'checkbox/lib/tests/test_fifo.py'
--- checkbox/lib/tests/test_fifo.py 2012-04-23 19:36:19 +0000
+++ checkbox/lib/tests/test_fifo.py 2012-05-02 10:18:20 +0000
@@ -26,7 +26,6 @@
26from unittest import TestCase26from unittest import TestCase
2727
28from checkbox.lib.fifo import fifo_create28from checkbox.lib.fifo import fifo_create
29from checkbox.lib.file import File
3029
3130
32class TestFifo(TestCase):31class TestFifo(TestCase):
@@ -39,9 +38,6 @@
39 os.unlink(self.path)38 os.unlink(self.path)
4039
41 def test_fifo_create(self):40 def test_fifo_create(self):
42 read_file = File()41 reader, writer = fifo_create(self.path)
43 write_file = File()42 self.assertNotEquals(reader.fileno(), -1)
4443 self.assertNotEquals(writer.fileno(), -1)
45 self.assertTrue(fifo_create(self.path, read_file, write_file))
46 self.assertNotEquals(read_file.fileno(), -1)
47 self.assertNotEquals(write_file.fileno(), -1)
4844
=== modified file 'checkbox/lib/tests/test_file.py'
--- checkbox/lib/tests/test_file.py 2012-04-23 19:36:19 +0000
+++ checkbox/lib/tests/test_file.py 2012-05-02 10:18:20 +0000
@@ -21,6 +21,7 @@
21__all__ = []21__all__ = []
2222
23import os23import os
24from io import (SEEK_SET, SEEK_CUR)
2425
25from unittest import TestCase26from unittest import TestCase
2627
@@ -30,7 +31,7 @@
30 )31 )
3132
32from checkbox.lib.file import (33from checkbox.lib.file import (
33 File,34 create_file,
34 fd_open,35 fd_open,
35 fd_close,36 fd_close,
36 )37 )
@@ -46,40 +47,15 @@
46 if os.path.exists(self.path):47 if os.path.exists(self.path):
47 os.unlink(self.path)48 os.unlink(self.path)
4849
49 def test_init(self):50 def test_create_file(self):
50 file = File()51 file = create_file(self.path)
51 self.assertFalse(file)52 self.assertTrue(file)
5253
53 def test_create_from_path(self):54 file = create_file(self.temp)
54 file = File.create_from_path(self.path)55 self.assertFalse(file)
55 self.assertEquals(file.errno, 0)56
56 self.assertTrue(file)57 file = create_file(0xFFFF)
5758 self.assertFalse(file)
58 file = File.create_from_path(self.temp)
59 self.assertEquals(file.errno, 2)
60 self.assertFalse(file)
61
62 def test_initialize_from_fd(self):
63 file = File.create_from_fd(self.fd)
64 self.assertEquals(file.errno, 0)
65 self.assertTrue(file)
66
67 file = File.create_from_fd(-1)
68 self.assertEquals(file.errno, 9)
69 self.assertFalse(file)
70
71 def test_reset(self):
72 file = File.create_from_path(self.path)
73 self.assertTrue(file)
74
75 file.reset()
76 self.assertFalse(file)
77
78 file = File.create_from_path(self.temp)
79 self.assertEquals(file.errno, 2)
80
81 file.reset()
82 self.assertEquals(file.errno, 0)
8359
84 def test_fd_open(self):60 def test_fd_open(self):
85 fd = fd_open(self.path, os.O_RDONLY)61 fd = fd_open(self.path, os.O_RDONLY)
@@ -94,3 +70,30 @@
9470
95 ret = fd_close(-1)71 ret = fd_close(-1)
96 self.assertEquals(ret, -1)72 self.assertEquals(ret, -1)
73
74 def test_tell_seek(self):
75 file = create_file(self.fd, 'w')
76 self.assertTrue(file)
77
78 self.assertEquals(file.tell(), 0)
79 pos = file.write('ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890')
80 self.assertEquals(pos, 36)
81 self.assertEquals(file.tell(), 36)
82 file.close()
83
84 file = create_file(self.path)
85 self.assertTrue(file)
86
87 self.assertEquals(file.tell(), 0)
88 str = file.read(4)
89 self.assertEquals(str, 'ABCD')
90 self.assertEquals(file.tell(), 4)
91
92 file.seek(14, SEEK_CUR)
93 self.assertEquals(file.tell(), 18)
94
95 os.lseek(file.fileno(), 28, SEEK_SET)
96 self.assertEquals(file.tell(), 28)
97 str = file.read(1)
98 self.assertEquals(str, '3')
99 self.assertEquals(file.tell(), 29)
97100
=== modified file 'checkbox/runner/manager.py'
--- checkbox/runner/manager.py 2012-04-23 19:36:19 +0000
+++ checkbox/runner/manager.py 2012-05-02 10:18:20 +0000
@@ -26,7 +26,7 @@
26import logging26import logging
2727
28from checkbox.lib import param28from checkbox.lib import param
29from checkbox.lib.file import File29from checkbox.lib.file import create_file
3030
31from checkbox.daemon.server import server31from checkbox.daemon.server import server
3232
@@ -214,9 +214,9 @@
214214
215 if self._output_record_is_stdout:215 if self._output_record_is_stdout:
216 logging.info("Will write output record to STDOUT")216 logging.info("Will write output record to STDOUT")
217 file = File.create_from_fd(1)217 file = create_file(1)
218 else:218 else:
219 file = File.create_from_path(self._output_record_path, "a")219 file = create_file(self._output_record_path, "a")
220 if not file:220 if not file:
221 logging.info(221 logging.info(
222 "Failed to open output record '%s': %s",222 "Failed to open output record '%s': %s",
223223
=== modified file 'checkbox/runner/starter.py'
--- checkbox/runner/starter.py 2012-04-23 19:36:19 +0000
+++ checkbox/runner/starter.py 2012-05-02 10:18:20 +0000
@@ -30,7 +30,7 @@
3030
31from checkbox.lib import param31from checkbox.lib import param
32from checkbox.lib.enum import Enum32from checkbox.lib.enum import Enum
33from checkbox.lib.file import File33from checkbox.lib.file import create_file
3434
35from checkbox.daemon.server import (35from checkbox.daemon.server import (
36 server,36 server,
@@ -190,7 +190,7 @@
190 self._execute_directory, "%d.recover" % server.pid)190 self._execute_directory, "%d.recover" % server.pid)
191191
192 tmp_path = "%s.tmp" % self._recovery_path192 tmp_path = "%s.tmp" % self._recovery_path
193 tmp_file = File.create_from_path(tmp_path, "w")193 tmp_file = create_file(tmp_path, "w")
194 if not tmp_file:194 if not tmp_file:
195 logging.info("Failed to open recovery file %s", tmp_path)195 logging.info("Failed to open recovery file %s", tmp_path)
196 return196 return

Subscribers

People subscribed via source and target branches

to all changes: