[vmchecker-dev] CR request: submit.py

[Lucian Adrian Grijincu]

Hi,

On Fri, Jul 10, 2009 at 1:04 PM, Alexandru Moșoi<brtzsnr at gmail.com> wrote:
> [0] http://github.com/brtzsnr/vmchecker/blob/c3c6eaca2300d8bf5309f0f46998ff21f1551b15/lib/submit.py
> [1] http://github.com/brtzsnr/vmchecker/blob/c3c6eaca2300d8bf5309f0f46998ff21f1551b15/tests/ut_submit.py

1. github sucks: it does not support CR for newly added files, just for diffs.
2. submit.py store() does two things:
  - add stuff to SQL
  - add stuff to filesystem
a) Please split them.
b) you *only* catch VmcheckerError. Which function throws this? What
happens if other functions throw (mainly the os. and sql. ones)?
c) you don't rollback the fs changes if an exception gets raised
I know there's this to cope with the problem:
        if os.path.lexists(location):
            # cleans old files left from a failed
            # transaction
            shutil.rmtree(location)
but shouldn't the filesystem be in sync with the SQL? I donno, just
seems better.
3.     def setUp(self):
        # creates a course
        # creates an assignment
        # creats an user
I see this more like functions on their own not comments in a large
function. It will help while writing other tests.

-- 
 .
..: Lucian