Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python3: Use bytes, instead of strings on Windows too. #15

Closed
wants to merge 3 commits into from
Closed

Python3: Use bytes, instead of strings on Windows too. #15

wants to merge 3 commits into from

Conversation

kraptor
Copy link

@kraptor kraptor commented Jan 12, 2016

Seems that a leftover when upgading the code to support Python 3, the Unix version was updated but not the windows side.

UNIX sIde already uses bytes:

    def _physical_pull(self):
        os.write(self.trigger, b'x')

The error spitted if this patch is not applied is:

Exception in thread Connect([(<AddressFamily.AF_INET: 2>, ['127.0.0.1', 4881])]):
Traceback (most recent call last):
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\threading.py", line 923, in _bootstrap_inner
    self.run()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 390, in run
    success = self.try_connecting(attempt_timeout)
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 421, in try_connecting
    r = self._connect_wrappers(wrappers, deadline)
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 503, in _connect_wrappers
    wrap.connect_procedure()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 589, in connect_procedure
    self.test_connection()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 597, in test_connection
    self.conn = ManagedClientConnection(self.sock, self.addr, self.mgr)
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\connection.py", line 713, in __init__
    self.call_from_thread()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\trigger.py", line 106, in pull_trigger
    self._physical_pull()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\trigger.py", line 235, in _physical_pull
    self.trigger.send('x')
TypeError: a bytes-like object is required, not 'str'

Seems that a leftover when upgading the code to support Python 3, the Unix version was updated but not the windows side.

UNIX SIde:
        def _physical_pull(self):
            os.write(self.trigger, b'x')

The error spitted if this patch is not applied is:

Exception in thread Connect([(<AddressFamily.AF_INET: 2>, ['127.0.0.1', 4881])]):
Traceback (most recent call last):
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\threading.py", line 923, in _bootstrap_inner
    self.run()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 390, in run
    success = self.try_connecting(attempt_timeout)
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 421, in try_connecting
    r = self._connect_wrappers(wrappers, deadline)
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 503, in _connect_wrappers
    wrap.connect_procedure()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 589, in connect_procedure
    self.test_connection()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\client.py", line 597, in test_connection
    self.conn = ManagedClientConnection(self.sock, self.addr, self.mgr)
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\connection.py", line 713, in __init__
    self.call_from_thread()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\trigger.py", line 106, in pull_trigger
    self._physical_pull()
  File "E:\Dropbox\Projects\Python-x64-3.5.0\lib\site-packages\ZEO\zrpc\trigger.py", line 235, in _physical_pull
    self.trigger.send('x')
TypeError: a bytes-like object is required, not 'str'
@@ -232,4 +232,4 @@ def _close(self):
self.trigger.close()

def _physical_pull(self):
self.trigger.send('x')
self.trigger.send(b'x')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are still supporting Python 3.2, which doesn't have the bytes literal prefix. You can use something like the following:

from ZODB._compat import ascii_bytes

TRIGGER_CHARACTER = ascii_bytes('x')

and then use that constant instead.

@mgedmin
Copy link
Member

mgedmin commented Jan 13, 2016

If winbot failed to notice the bug, perhaps we also need a new unit test?

@mgedmin
Copy link
Member

mgedmin commented Jan 13, 2016

D'oh, winbot is not running any tests on Python 3.

I suggest we start using Appveyor.

@agroszer
Copy link

winbot: sadly tox isn't helpful there

@kraptor
Copy link
Author

kraptor commented Jan 13, 2016

Fixed this with @tseaver comments both in posix and windows brachs of the code.

See https://github.com/davidanes/ZEO/commit/0d099c60082fd75c0f1a58fe842e40626bba06d5

Fixed this with @tseaver comments both in posix and windows brachs of the code.
@kraptor
Copy link
Author

kraptor commented Jan 13, 2016

Updated patch branch with commit in prev. comment, and update the pull request.

Incorrect import was used.
@kraptor
Copy link
Author

kraptor commented Jan 13, 2016

I don't know why this is failing now in the tests, will see later at home. We should review all code in zrpc for str/bytes and check that everything is right, IMHO.

@kraptor kraptor closed this Jan 20, 2016
@kraptor kraptor deleted the patch-1 branch January 20, 2016 17:58
@kraptor kraptor restored the patch-1 branch January 20, 2016 18:17
@kraptor kraptor deleted the patch-1 branch January 20, 2016 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants