SG-11735 Python 3 Support #200
Conversation
| import os | ||
| import re | ||
| from mock import patch, Mock, MagicMock | ||
| from .mock import patch, Mock, MagicMock |
| except httplib2.RedirectMissingLocation: | ||
| pass | ||
| except Exception, e: | ||
| except Exception as e: |
There was a problem hiding this comment.
local variable 'e' is assigned to but never used
| except httplib2.RedirectLimit: | ||
| pass | ||
| except Exception, e: | ||
| except Exception as e: |
There was a problem hiding this comment.
local variable 'e' is assigned to but never used
tests/httplib2test.py
Outdated
| uri = urlparse.urljoin(base, "301/onestep.asis") | ||
| destination = urlparse.urljoin(base, "302/final-destination.txt") | ||
| uri = six.moves.urllib.parse.urljoin(base, "301/onestep.asis") | ||
| destination = six.moves.urllib.parse.urljoin(base, "302/final-destination.txt") |
There was a problem hiding this comment.
local variable 'destination' is assigned to but never used
| e.g. d {'foo' : 'bar'} changed to {'foo': {"value": 'bar'}] | ||
| """ | ||
| return dict([(k, {key_name: v}) for (k,v) in (d or {}).iteritems()]) | ||
| return dict([(k, {key_name: v}) for (k,v) in six.iteritems((d or {}))]) |
shotgun_api3/shotgun.py
Outdated
| auth_string = "" | ||
| proxy_addr = "http://%s%s:%d" % (auth_string, self.config.proxy_server, self.config.proxy_port) | ||
| self.config.proxy_handler = urllib2.ProxyHandler({self.config.scheme : proxy_addr}) | ||
| self.config.proxy_handler = urllib.request.ProxyHandler({self.config.scheme : proxy_addr}) |
shotgun_api3/shotgun.py
Outdated
|
|
||
|
|
||
| # mimetypes imported in version specific imports | ||
| mimetypes.add_type('video/webm','.webm') # webm and mp4 seem to be missing |
| return | ||
|
|
||
| def write(data): | ||
| if not isinstance(data, basestring): |
| # Workaround for standalone backslash | ||
|
|
||
| def u(s): | ||
| return unicode(s.replace(r'\\', r'\\\\'), "unicode_escape") |
| def __dir__(self): | ||
| return ['parse', 'error', 'request', 'response', 'robotparser'] | ||
|
|
||
| _importer._add_module(Module_six_moves_urllib(__name__ + ".moves.urllib"), |
There was a problem hiding this comment.
expected 2 blank lines after class or function definition, found 1
| return None | ||
| get_source = get_code # same as get_code | ||
|
|
||
| _importer = _SixMetaPathImporter(__name__) |
There was a problem hiding this comment.
expected 2 blank lines after class or function definition, found 1
shotgun_api3/lib/six/__init__.py
Outdated
| class_types = (type, types.ClassType) | ||
| text_type = unicode | ||
| binary_type = str | ||
| file_types = (file, io.IOBase) |
| guess = guess_extension(gtype, strict) | ||
| if not guess: print "I don't know anything about type", gtype | ||
| else: print guess | ||
| if not guess: print("I don't know anything about type {}".format(gtype)) |
There was a problem hiding this comment.
multiple statements on one line (colon)
| print USAGE | ||
| if msg: print msg | ||
| print(USAGE) | ||
| if msg: print(msg) |
There was a problem hiding this comment.
multiple statements on one line (colon)
| self.__negotiatehttp(destpair[0], destpair[1]) | ||
| else: | ||
| self.__httptunnel = False | ||
| elif self.__proxy[0] == None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is None:'
| _orgsocket.connect(self, (self.__proxy[1], portnum)) | ||
| self.__negotiatehttp(destpair[0], destpair[1]) | ||
| elif self.__proxy[0] == PROXY_TYPE_HTTP_NO_TUNNEL: | ||
| if self.__proxy[2] != None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
| _orgsocket.connect(self, (self.__proxy[1], portnum)) | ||
| self.__negotiatesocks4(destpair[0], destpair[1]) | ||
| elif self.__proxy[0] == PROXY_TYPE_HTTP: | ||
| if self.__proxy[2] != None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
| raise ShotgunError("Could not upload file successfully, but "\ | ||
| "not sure why.\nPath: %s\nUrl: %s\nError: %s" % ( | ||
| path, url, str(result))) | ||
| path, url, result)) |
There was a problem hiding this comment.
continuation line missing indentation or outdented
| _orgsocket.connect(self, (self.__proxy[1], portnum)) | ||
| self.__negotiatesocks5(destpair[0], destpair[1]) | ||
| elif self.__proxy[0] == PROXY_TYPE_SOCKS4: | ||
| if self.__proxy[2] != None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
| ): | ||
| raise GeneralProxyError((5, _generalerrors[5])) | ||
| if self.__proxy[0] == PROXY_TYPE_SOCKS5: | ||
| if self.__proxy[2] != None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
| (not type(destpair) in (list, tuple)) | ||
| or (len(destpair) < 2) | ||
| or (not isinstance(destpair[0], (str, bytes))) | ||
| or (type(destpair[1]) != int) |
| if ( | ||
| (not type(destpair) in (list, tuple)) | ||
| or (len(destpair) < 2) | ||
| or (not isinstance(destpair[0], (str, bytes))) |
| req | ||
| + chr(0x03).encode() | ||
| + chr(len(destaddr)).encode() | ||
| + destaddr.encode() |
| req = ( | ||
| req | ||
| + chr(0x03).encode() | ||
| + chr(len(destaddr)).encode() |
| ipaddr = None | ||
| req = ( | ||
| req | ||
| + chr(0x03).encode() |
| + chr(len(self.__proxy[4])) | ||
| + self.__proxy[4] | ||
| + chr(len(self.__proxy[5])) | ||
| + self.__proxy[5] |
| chr(0x01).encode() | ||
| + chr(len(self.__proxy[4])) | ||
| + self.__proxy[4] | ||
| + chr(len(self.__proxy[5])) |
| UserCredentialsNotAllowedForSSOAuthenticationFault, | ||
| ProtocolError, ResponseError, Error, __version__) | ||
| from shotgun import SG_TIMEZONE as sg_timezone | ||
| from .shotgun import SG_TIMEZONE as sg_timezone |
There was a problem hiding this comment.
'.shotgun.SG_TIMEZONE as sg_timezone' imported but unused
| @@ -1,6 +1,7 @@ | |||
| from shotgun import (Shotgun, ShotgunError, ShotgunFileDownloadError, Fault, | |||
| from __future__ import absolute_import | |||
| from .shotgun import (Shotgun, ShotgunError, ShotgunFileDownloadError, Fault, | |||
There was a problem hiding this comment.
'.shotgun.AuthenticationFault' imported but unused
'.shotgun.Error' imported but unused
'.shotgun.Fault' imported but unused
'.shotgun.MissingTwoFactorAuthenticationFault' imported but unused
'.shotgun.ProtocolError' imported but unused
'.shotgun.ResponseError' imported but unused
'.shotgun.Shotgun' imported but unused
'.shotgun.ShotgunError' imported but unused
'.shotgun.ShotgunFileDownloadError' imported but unused
'.shotgun.UserCredentialsNotAllowedForSSOAuthenticationFault' imported but unused
'.shotgun.version' imported but unused
| and not "if-none-match" in headers | ||
| ): | ||
| headers["if-none-match"] = info["etag"] | ||
| if "last-modified" in info and not "last-modified" in headers: |
There was a problem hiding this comment.
test for membership should be 'not in'
| if ( | ||
| "etag" in info | ||
| and not self.ignore_etag | ||
| and not "if-none-match" in headers |
There was a problem hiding this comment.
line break before binary operator
test for membership should be 'not in'
| if entry_disposition == "STALE": | ||
| if ( | ||
| "etag" in info | ||
| and not self.ignore_etag |
| cached_value | ||
| and method in ["GET", "HEAD"] | ||
| and self.cache | ||
| and "range" not in headers |
| if ( | ||
| cached_value | ||
| and method in ["GET", "HEAD"] | ||
| and self.cache |
| del headers["if-modified-since"] | ||
| if ( | ||
| "authorization" in headers | ||
| and not self.forward_authorization_headers |
| if "location" in response: | ||
| location = response["location"] | ||
| (scheme, authority, path, query, fragment) = parse_uri(location) | ||
| if authority == None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is None:'
| if ( | ||
| self.follow_all_redirects | ||
| or (method in ["GET", "HEAD"]) | ||
| or response.status == 303 |
|
|
||
| if ( | ||
| self.follow_all_redirects | ||
| or (method in ["GET", "HEAD"]) |
| ) | ||
| ) | ||
| ) | ||
| except (ssl.SSLError, ssl.CertificateError) as e: |
There was a problem hiding this comment.
local variable 'e' is assigned to but never used
| # Python 3.3 compatibility: emulate the check_hostname behavior | ||
| if ( | ||
| not hasattr(self._context, "check_hostname") | ||
| and not self.disable_ssl_certificate_validation |
| return ( | ||
| "<ProxyInfo type={p.proxy_type} " | ||
| "host:port={p.proxy_host}:{p.proxy_port} rdns={p.proxy_rdns}" | ||
| + " user={p.proxy_user} headers={p.proxy_headers}>" |
| ) | ||
|
|
||
| def isgood(self): | ||
| return socks and (self.proxy_host != None) and (self.proxy_port != None) |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
| def request(self, method, request_uri, headers, content, cnonce=None): | ||
| """Modify the request headers""" | ||
| H = lambda x: _md5(x.encode("utf-8")).hexdigest() | ||
| KD = lambda s, d: H("%s:%s" % (s, d)) |
There was a problem hiding this comment.
do not assign a lambda expression, use a def
|
|
||
| def request(self, method, request_uri, headers, content, cnonce=None): | ||
| """Modify the request headers""" | ||
| H = lambda x: _md5(x.encode("utf-8")).hexdigest() |
There was a problem hiding this comment.
do not assign a lambda expression, use a def
| if ( | ||
| "etag" in info | ||
| and not self.ignore_etag | ||
| and not "if-none-match" in headers |
There was a problem hiding this comment.
line break before binary operator
test for membership should be 'not in'
| if entry_disposition == "STALE": | ||
| if ( | ||
| "etag" in info | ||
| and not self.ignore_etag |
| cached_value | ||
| and method in ["GET", "HEAD"] | ||
| and self.cache | ||
| and "range" not in headers |
| if ( | ||
| cached_value | ||
| and method in ["GET", "HEAD"] | ||
| and self.cache |
|
|
||
| if ( | ||
| cached_value | ||
| and method in ["GET", "HEAD"] |
| """ | ||
| LOG.setLevel(logging.WARN) | ||
|
|
||
| def _is_mimetypes_broken(): |
| self.sendall( | ||
| chr(0x01).encode() | ||
| + chr(len(self.__proxy[4])) | ||
| + self.__proxy[4] |
| # authentication. | ||
| self.sendall( | ||
| chr(0x01).encode() | ||
| + chr(len(self.__proxy[4])) |
| Negotiates a connection through a SOCKS5 server. | ||
| """ | ||
| # First we'll send the authentication packages we support. | ||
| if (self.__proxy[4] != None) and (self.__proxy[5] != None): |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
| hdrs.remove(endpt) | ||
| host = host.split(" ")[1] | ||
| endpt = endpt.split(" ") | ||
| if self.__proxy[4] != None and self.__proxy[5] != None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
|
|
||
| if ( | ||
| method in self.optimistic_concurrency_methods | ||
| and self.cache |
| del headers["if-modified-since"] | ||
| if ( | ||
| "authorization" in headers | ||
| and not self.forward_authorization_headers |
| if "location" in response: | ||
| location = response["location"] | ||
| (scheme, authority, path, query, fragment) = parse_uri(location) | ||
| if authority == None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is None:'
| if ( | ||
| self.follow_all_redirects | ||
| or (method in ["GET", "HEAD"]) | ||
| or response.status == 303 |
|
|
||
| if ( | ||
| self.follow_all_redirects | ||
| or (method in ["GET", "HEAD"]) |
willis102
force-pushed
the
SG-11735_python_3_support
branch
from
6b049b9 to
2f17f06
Compare
| self.connections = {} | ||
| # The location of the cache, for now a directory | ||
| # where cached responses are held. | ||
| if cache and isinstance(cache, basestring): |
| server_software = os.environ.get('SERVER_SOFTWARE', '') | ||
| if (server_software.startswith('Google App Engine/') or | ||
| server_software.startswith('Development/') or | ||
| server_software.startswith('testutil/')): |
There was a problem hiding this comment.
visually indented line with same indent as next logical line
| def is_gae_instance(): | ||
| server_software = os.environ.get('SERVER_SOFTWARE', '') | ||
| if (server_software.startswith('Google App Engine/') or | ||
| server_software.startswith('Development/') or |
| # Use a different connection object for Google App Engine Standard Environment. | ||
| def is_gae_instance(): | ||
| server_software = os.environ.get('SERVER_SOFTWARE', '') | ||
| if (server_software.startswith('Google App Engine/') or |
| raise | ||
| except (socket.timeout, socket.gaierror): | ||
| raise | ||
| except socket.error as msg: |
There was a problem hiding this comment.
local variable 'msg' is assigned to but never used
| def request(self, method, request_uri, headers, content, cnonce=None): | ||
| """Modify the request headers""" | ||
| H = lambda x: _md5(x).hexdigest() | ||
| KD = lambda s, d: H("%s:%s" % (s, d)) |
There was a problem hiding this comment.
do not assign a lambda expression, use a def
|
|
||
| def request(self, method, request_uri, headers, content, cnonce=None): | ||
| """Modify the request headers""" | ||
| H = lambda x: _md5(x).hexdigest() |
There was a problem hiding this comment.
do not assign a lambda expression, use a def
|
|
||
| def depth(self, request_uri): | ||
| (scheme, authority, path, query, fragment) = parse_uri(request_uri) | ||
| return request_uri[len(self.path) :].count("/") |
| freshness_lifetime = 0 | ||
| elif "expires" in response_headers: | ||
| expires = email.Utils.parsedate_tz(response_headers["expires"]) | ||
| if None == expires: |
There was a problem hiding this comment.
comparison to None should be 'if cond is None:'
|
|
||
| if ( | ||
| "pragma" in request_headers | ||
| and request_headers["pragma"].lower().find("no-cache") != -1 |
| # Test unicode mixed with utf-8 as reported in Ticket #17959 | ||
| d = { "results" : ["foo", "bar"] } | ||
| a = { "utf_str": "\xe2\x88\x9a", "unicode_str": "\xe2\x88\x9a".decode("utf-8") } | ||
| a = { "utf_str": "\xe2\x88\x9a", "unicode_str": six.ensure_text("\xe2\x88\x9a") } |
There was a problem hiding this comment.
whitespace after '{'
whitespace before '}'
| # In Python2, make sure that non-utf-8 encoded paths raise when they | ||
| # can't be converted to utf-8. For Python3, we'll skip these tests | ||
| # since string encoding is handled differently. | ||
| #TODO: do we need to add an equivalent test here?? |
There was a problem hiding this comment.
block comment should start with '# '
| # be floating around. Therefore, we can't use isinstance() to check for | ||
| # the six meta path importer, since the other six instance will have | ||
| # inserted an importer with different class. | ||
| if (type(importer).__name__ == "_SixMetaPathImporter" and |
|
|
||
|
|
||
|
|
||
| def python_2_unicode_compatible(klass): |
| break | ||
| if want_unicode: | ||
| newline = unicode("\n") | ||
| space = unicode(" ") |
| return | ||
|
|
||
| def write(data): | ||
| if not isinstance(data, basestring): |
| # Workaround for standalone backslash | ||
|
|
||
| def u(s): | ||
| return unicode(s.replace(r'\\', r'\\\\'), "unicode_escape") |
| def __dir__(self): | ||
| return ['parse', 'error', 'request', 'response', 'robotparser'] | ||
|
|
||
| _importer._add_module(Module_six_moves_urllib(__name__ + ".moves.urllib"), |
There was a problem hiding this comment.
expected 2 blank lines after class or function definition, found 1
| return None | ||
| get_source = get_code # same as get_code | ||
|
|
||
| _importer = _SixMetaPathImporter(__name__) |
There was a problem hiding this comment.
expected 2 blank lines after class or function definition, found 1
| string_types = basestring, | ||
| integer_types = (int, long) | ||
| class_types = (type, types.ClassType) | ||
| text_type = unicode |
|
|
||
| # Whitelist certain data types for order_field, since no_sorting is not | ||
| # currently exposed. These should be good bets to be sortable. | ||
| allow_sorting_types = ("number", "date") |
There was a problem hiding this comment.
local variable 'allow_sorting_types' is assigned to but never used
| MAXSIZE = sys.maxsize | ||
| else: | ||
| string_types = basestring, | ||
| integer_types = (int, long) |
|
|
||
| MAXSIZE = sys.maxsize | ||
| else: | ||
| string_types = basestring, |
| if six.PY3: | ||
| file_types = (io.IOBase, ) | ||
| else: | ||
| file_types = (file, io.IOBase) |
|
|
||
|
|
||
| if ssl is None: | ||
| _ssl_wrap_socket = _ssl_wrap_socket_unsupported |
There was a problem hiding this comment.
redefinition of unused '_ssl_wrap_socket' from line 78
| else: | ||
| raise AttributeError(name) | ||
| from .python2 import * | ||
| from .python2 import socks # ensure include in namespace |
There was a problem hiding this comment.
'.python2.socks' imported but unused
| return self | ||
| else: | ||
| raise AttributeError(name) | ||
| from .python2 import * |
There was a problem hiding this comment.
'.python2.*' imported but unused
'from .python2 import *' used; unable to detect undefined names
| # import the proper implementation into the module namespace depending on the | ||
| # current python version | ||
| if six.PY3: | ||
| from .python3 import * |
There was a problem hiding this comment.
'.python3.*' imported but unused
'from .python3 import *' used; unable to detect undefined names
Python 3 compatability pass. Switch from endswith to contains. Fix tests.
Since we're now targeting Python 2.6+, we can use the built-in json module.
willis102
force-pushed
the
SG-11735_python_3_support
branch
from
54a9f5e to
7f57691
Compare
willis102
force-pushed
the
SG-11735_python_3_support
branch
from
7f57691 to
430bb7f
Compare
| uri = urlparse.urljoin(base, "301/onestep.asis") | ||
| destination = urlparse.urljoin(base, "302/final-destination.txt") | ||
| uri = urllib.parse.urljoin(base, "301/onestep.asis") | ||
| destination = urllib.parse.urljoin(base, "302/final-destination.txt") |
There was a problem hiding this comment.
local variable 'destination' is assigned to but never used
willis102
force-pushed
the
SG-11735_python_3_support
branch
from
430bb7f to
f622bd4
Compare
Pull Request Test Coverage Report for Build 1096
💛 - Coveralls |
First pass at adding Python 3 support for Python API. For CI and review.
Use six module to provide compatibility with Python 2.6, 2.7 and 3.7.