fix upload fileds with uploadfs/pyfilesystem. Added test.

[michele-comitini]

@mdipierro @gi0baro The upload code in present pyDAL is uncovered. I made a test to correct a bug that shows with uploadfs=pyfilesystem. This bug affects current web2py too.

[michele-comitini]

Python3 has problems with the str <-> bytes. The code needs some upgrade

[gi0baro]

@michele-comitini you can import StringIO from the _compat module. See https://github.com/web2py/pydal/wiki/Write-py2py3-compatible-code

[michele-comitini]

OK thanks for the pointer I'll try to fix it.
Il 15/apr/2016 17:56, "Giovanni Barillari" notifications@github.com ha
scritto:

@michele-comitini https://github.com/michele-comitini you can import
StringIO from the _compat module. See
https://github.com/web2py/pydal/wiki/Write-py2py3-compatible-code

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#342 (comment)

[michele-comitini]

@gi0baro rebased on master. Should work now.

[michele-comitini]

@niphlod what's the problem with codecov and appveyor??

[michele-comitini]

@gi0baro can you check why appveyor and codecov fail? The code does not fail locally
tnx

[gi0baro]

@michele-comitini seems windows-only related:

ERROR: testUploadField (tests.sql.TestFields)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests\sql.py", line 178, in testUploadField
    row.delete_record()
  File "pydal\helpers\classes.py", line 259, in __call__
    return self.db(self.db[self.tablename]._id == self.id).delete()
  File "pydal\objects.py", line 2028, in delete
    if any(f(self) for f in table._before_delete):
  File "pydal\objects.py", line 2028, in <genexpr>
    if any(f(self) for f in table._before_delete):
  File "pydal\objects.py", line 2125, in delete_uploaded_files
    os.unlink(oldpath)
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: 'c:\\users\\appveyor\\appdata\\local\\temp\\1\\tt.fileobj.9ff9edd71c2596c9.746d706c70666a6276.txt

Don't know if is really windows-related or just something bad with appveyor. We should wait for @niphlod , maybe he can help.

[niphlod]

I can test at home but rarely appveyor is faulty: its biggest con (slowliness) brings pretty much the greatest pro of being a pristine env equal to a normal host.

[michele-comitini]

@niphlod did you discover anything?

[niphlod]

nope, sorry, didn't have time. maybe this evening I'll get some ;-(

[michele-comitini]

@gi0baro @niphlod bump!
if there is a problem with that code it points to a problem with current code in upload fields, does not depend on my patches.

[niphlod]

@michele-comitini : I can only confirm it's indeed happening and it's not an appveyor problem... investigating

[niphlod]

confirming also that backporting the test and running that test BEFORE the fix still shows the error. @michele-comitini : you're not the one to blame :-P
Now let's try to understand why the heck the file seems locked.

[niphlod]

aha, pinpointed! @michele-comitini it has to do on how you written the tests... or maybe @gi0baro can shed some light. Fortunately windows is more picky than linux. The fact is that retrieve() returns - correctly - an open handle to the file (so you can fetch its content).
But! ... unless you call retr_stream.close() before delete()ing the record, the handle is still open and you can't delete the file.

IMHO the behaviour is correct, but lesser-skilled newbies are going to leave open handles everywhere (dunno if this means leakage in linux).
I very much dislike web2py's behaviour of not ever using "with" anywhere in the code (thing that maybe pushed to write tests without "with" also) but in this case it's the user's responsibility to close the handle before trying to delete something you just retrieve()d

[michele-comitini]

OK many thanks @niphlod! On *nix like systems deleting a file can be done anytime. That is the way temporary files are often dealt with. i.e. as soon as the file has been opened, it is also unlinked, when the stream is closed or the process dies, the stream ceases to exist and no reference is left around on the fs. Cool I didn't know that windows behaved differently so I didn't care of closing the stream!

[codecov-io]

Current coverage is 67.50%

Merging #342 into master will increase coverage by +0.50% as of ab55599

Powered by Codecov. Updated on successful CI builds.