Skip to content

aggr changes#2

Open
avi-slavkin wants to merge 4 commits intopython3from
python3_aggr
Open

aggr changes#2
avi-slavkin wants to merge 4 commits intopython3from
python3_aggr

Conversation

@avi-slavkin
Copy link

No description provided.

@@ -1,4 +1,6 @@
from haigha2.connection import Connection
from six import PY2
Copy link

Choose a reason for hiding this comment

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

where is this used? I am not sure I understand what changed in this file

###

def connect(self, address):
def connect(self, host_port_tuple):
Copy link

Choose a reason for hiding this comment

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

why we renamed the argument to host_port_tuple?
the connect interface useually pass address and lets the underline layer to handle the address format
https://docs.python.org/2/library/socket.html#socket.socket.connect

if six.PY2:
sslobj = self._context._wrap_socket(self._sock, server_side, ssl_sock=self, server_hostname=server_hostname)
else:
context = self.context if PY33 else self._context
Copy link

Choose a reason for hiding this comment

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

why we need to check PY33? what changed in python3.3? (add comment about it)

try:
# This is added in Python 3.5, http://bugs.python.org/issue21965
SSLObject = ssl.SSLObject
except NameError:
Copy link

Choose a reason for hiding this comment

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

do we expect NameError and not attribute error?

In [49]: import ssl

In [50]: ssl.SSLObject
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-50-f8284913d90b> in <module>()
----> 1 ssl.SSLObject

AttributeError: 'module' object has no attribute 'SSLObject'```

avi-slavkin and others added 3 commits May 27, 2019 13:55
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.

3 participants