-
Notifications
You must be signed in to change notification settings - Fork 70
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
Removed reference to python2 #262
Changes from 3 commits
9cd1d5b
926fd84
0132bc7
871a0a1
81b7a38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,9 +56,9 @@ | |
from http.client import HTTPConnection, HTTPSConnection | ||
from http.cookies import SimpleCookie | ||
except ImportError: | ||
# Use Python 2.7 import as a fallback | ||
from httplib import HTTPConnection, HTTPSConnection | ||
from Cookie import SimpleCookie | ||
# Throw incompatible version error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
print("Could not find a suitable import for HTTPConnection, HTTPSConnection and SimpleCookie. Please install or upgrade to a higher version that is supported by Python 3.7 or higher.") | ||
|
||
|
||
from pyeapi.utils import make_iterable | ||
|
||
|
@@ -250,7 +250,7 @@ def connect(self): | |
|
||
Redefined/copied and extended from httplib.py:1105 (Python 2.6.x). | ||
This is needed to pass cert_reqs=ssl.CERT_REQUIRED as parameter | ||
to ssl.wrap_socket(), which forces SSL to check server certificate | ||
to ssl.wrap_socket()(Now changed to ssl.SSLContext.wrap_socket() as the former has been deprecated from python 3.7), which forces SSL to check server certificate | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you break the formatting and coding style here. flake might complain and doc building might have too. Please adjust accordingly. Also, be accurate with spaces, as it all goes into the official documentation |
||
against our client certificate. | ||
""" | ||
sock = socket.create_connection((self.host, self.port), self.timeout) | ||
|
@@ -259,11 +259,11 @@ def connect(self): | |
self._tunnel() | ||
# If there's no CA File, don't force Server Certificate Check | ||
if self.ca_file: | ||
self.sock = ssl.wrap_socket(sock, self.key_file, self.cert_file, | ||
self.sock = ssl.SSLContext.wrap_socket(sock, self.key_file, self.cert_file, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, however it now breaks the formatting rules (the line extends past allowed column size), needs some adjustment |
||
ca_certs=self.ca_file, | ||
cert_reqs=ssl.CERT_REQUIRED) | ||
else: | ||
self.sock = ssl.wrap_socket(sock, self.key_file, | ||
self.sock = ssl.SSLContext.wrap_socket(sock, self.key_file, | ||
self.cert_file, | ||
cert_reqs=ssl.CERT_NONE) | ||
|
||
|
@@ -309,8 +309,7 @@ def authentication(self, username, password): | |
_auth_bin = base64.encodebytes(_auth_text.encode()) | ||
_auth = _auth_bin.decode() | ||
else: | ||
# For Python 2.7 | ||
_auth = str(base64.encodestring(_auth_text)) | ||
raise Exception("Incompatible python version.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This handling is also superfluous now - because it's all will be guarded by import exception (i.e. if python 2 is used), so again, the |
||
_auth = _auth.replace('\n', '') | ||
self._auth = ("Authorization", "Basic %s" % _auth) | ||
|
||
|
@@ -449,10 +448,11 @@ def send(self, data): | |
|
||
self.transport.endheaders(message_body=data) | ||
|
||
try: # Python 2.7: use buffering of HTTP responses | ||
response = self.transport.getresponse(buffering=True) | ||
except TypeError: # Python 2.6: older, and 3.x on | ||
try: #For Python 3.x+ | ||
response = self.transport.getresponse() | ||
except TypeError: #For < Python3.x | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
print("Incompatible python version") | ||
|
||
|
||
response_content = response.read() | ||
_LOGGER.debug('Response: status:{status}, reason:{reason}'.format( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,8 +43,8 @@ | |
# Try Python 3.x import first | ||
from itertools import zip_longest | ||
except ImportError: | ||
# Use Python 2.7 import as a fallback | ||
from itertools import izip_longest as zip_longest | ||
# Throw incompatible version error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and here. |
||
print("A supported version of itertools is missing. Please upgrade to a higher version that is supported by Python 3.7 or higher.") | ||
|
||
_LOGGER = logging.getLogger(__name__) | ||
_LOGGER.setLevel(logging.DEBUG) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look, you print here an error and let parsing of the module continue. In 99.99% case nothing terribly wrong would happen (it would end up throwing another exception), but it might have side effects. A bit better handling would be re-raising the exception, but then printing an error does not make much sense - python tracebacks are typically very sufficient. Thus, the best handling (here and in other places) - just remove the
try
/except
block and let python throw an exception.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Better to remove it as we don't need the fallback at all