atomic_rename failure on Windows Python2.7

[Murray-LIANG]

I found below error during the CI test of storops. It reported by storops appveyor tests: https://ci.appveyor.com/project/emc-openstack/storops/build/job/y6tctpnpe54j4is0

I am just wondering whether you met this before on your Windows Py27 testing?

The version of persist-queue is persist-queue==0.4.1.

___________________ TestPQueue.test_enqueue_expected_error ____________________
args = (<storops_test.lib.test_tasks.TestPQueue testMethod=test_enqueue_expected_error>,)
    @functools.wraps(func)
    @patch(target='storops.vnx.navi_command.'
                  'NaviCommand.execute_naviseccli',
           new=cli.mock_execute)
    def func_wrapper(*args):
>       return func(*args)
storops_test\vnx\cli_mock.py:106: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
storops_test\lib\test_tasks.py:96: in test_enqueue_expected_error
    self.q.put(fake_vnx.delete_hba, hba_uid=uid)
storops\lib\tasks.py:47: in put
    self._q.put_nowait(item)
.tox\py27\lib\site-packages\persistqueue\queue.py:185: in put_nowait
    return self.put(item, False)
.tox\py27\lib\site-packages\persistqueue\queue.py:161: in put
    self._put(item)
.tox\py27\lib\site-packages\persistqueue\queue.py:182: in _put
    self._saveinfo()
.tox\py27\lib\site-packages\persistqueue\queue.py:274: in _saveinfo
    atomic_rename(tmpfn, self._infopath())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src = 'c:\\users\\appveyor\\appdata\\local\\temp\\1\\tmperonkh'
dst = 'c:\users\appveyor\appdata\local\temp\1\tmpuqdetsstorops\info'
    def atomic_rename(src, dst):
        try:
            os.replace(src, dst)
        except AttributeError:  # python < 3.3
            import sys
    
            if sys.platform == 'win32':
                import ctypes
    
                if sys.version_info[0] == 2:
                    _str = unicode  # noqa
                    _bytes = str
                else:
                    _str = str
                    _bytes = bytes
    
                if isinstance(src, _str) and isinstance(dst, _str):
                    MoveFileEx = ctypes.windll.kernel32.MoveFileExW
                elif isinstance(src, _bytes) and isinstance(dst, _bytes):
                    MoveFileEx = ctypes.windll.kernel32.MoveFileExA
                else:
>                   raise ValueError("Both args must be bytes or unicode.")
E                   ValueError: Both args must be bytes or unicode.
.tox\py27\lib\site-packages\persistqueue\queue.py:44: ValueError
____________________ TestPQueue.test_enqueue_storops_error ____________________

[peter-wangxu]

py27 was passed on CI: https://ci.appveyor.com/project/peter-wangxu/persist-queue/builds/19169922

what's the type of src and dst?

[Murray-LIANG]

src = 'c:\\users\\liangr\\appdata\\local\\temp\\tmpol0feq'
dst = 'c:\users\liangr\appdata\local\temp\tmpsulfcsstorops\info'

('type(src)', <type 'str'>)
('type(dst)', <type 'unicode'>)
('isinstance(src, _str)', False)
('isinstance(dst, _str)', True)
('isinstance(src, _bytes)', True)
('isinstance(dst, _bytes)', False)

[Murray-LIANG]

Most of storops python source files set from __future__ import unicode_literals. This would make any string literal to be unicode.

In TestPQueue of storops,

from __future__ import unicode_literals
# ... ...
self.path = tempfile.mkdtemp(suffix='storops')
self.q = tasks.PQueue(self.path)

And this would make _infopath in Persist-Queue and dst of atomic_rename to be unicode.

While in Persist-Queue, unicode_literals is not imported. And src of atomic_rename is returned by tempfile.mkstemp(), so it is str.

That is the reason what we got as the above comments.
src is str, dst is unicode.

So the conclusion is that any source codes with unicode_literals imported could meet this issue.

Can we convert the type of src or dst to make them as the same type first?

[peter-wangxu]

@Murray-LIANG loooooking forward to your change:)

[Murray-LIANG]

Sorry for the late response. I will do the fix next days. My proposal is that converting the src and dst to str or unicode first regardless of their original types.

What I don't understand is that why it chooses different ctypes move functions based on the type of file paths?

[peter-wangxu]

AFAIK, this is the windows API limitation, MoveFileExW for unicode MoveFileExA for ascii only.
Agreed with you to convert, and we can use the former function for both str or unicode.

[peter-wangxu]

@Murray-LIANG any update?