From a0f01cf9cf5521fd034813242727de72eb52df42 Mon Sep 17 00:00:00 2001 From: Steve Kowalik Date: Wed, 28 Aug 2019 15:24:54 +1000 Subject: [PATCH] Remove more Python version specific code (#1193) Remove a bunch of other call sites that behaved differently between Python 2 and 3, massively cleaning up a few messy methods. --- github/GithubException.py | 4 +++- github/GithubObject.py | 16 +++++++--------- github/MainClass.py | 5 +---- github/Repository.py | 35 +++++++++++----------------------- github/Requester.py | 5 ----- tests/Exceptions.py | 18 +++--------------- tests/Framework.py | 40 +++++++++++++-------------------------- tests/Issue494.py | 4 ++-- tests/Search.py | 11 ++++------- 9 files changed, 44 insertions(+), 94 deletions(-) diff --git a/github/GithubException.py b/github/GithubException.py index f12828ee..0186c690 100644 --- a/github/GithubException.py +++ b/github/GithubException.py @@ -30,6 +30,8 @@ # # ################################################################################ +import json + class GithubException(Exception): """ @@ -59,7 +61,7 @@ class GithubException(Exception): return self.__data def __str__(self): - return str(self.status) + " " + str(self.data) + return "{status} {data}".format(status=self.status, data=json.dumps(self.data)) class BadCredentialsException(GithubException): diff --git a/github/GithubObject.py b/github/GithubObject.py index d78bad56..f6a817aa 100644 --- a/github/GithubObject.py +++ b/github/GithubObject.py @@ -33,7 +33,6 @@ ################################################################################ from __future__ import absolute_import -import sys import datetime from operator import itemgetter @@ -41,8 +40,6 @@ from . import GithubException from . import Consts import six -atLeastPython3 = sys.hexversion >= 0x03000000 - class _NotSetType: def __repr__(self): @@ -233,13 +230,14 @@ class GithubObject(object): def format_params(params): items = list(params.items()) for k, v in sorted(items, key=itemgetter(0), reverse=True): - isText = isinstance(v, (str, six.text_type)) - if isText and not atLeastPython3: - v = v.encode('utf-8') - yield '{k}="{v}"'.format(k=k, v=v) if isText else '{k}={v}'.format(k=k, v=v) - return '{class_name}({params})'.format( + if isinstance(v, bytes): + v = v.decode('utf-8') + if isinstance(v, six.text_type): + v = u'"{v}"'.format(v=v) + yield u'{k}={v}'.format(k=k, v=v) + return u'{class_name}({params})'.format( class_name=self.__class__.__name__, - params=", ".join(list(format_params(params))) + params=u", ".join(list(format_params(params))) ) diff --git a/github/MainClass.py b/github/MainClass.py index afdf9d46..f377f496 100644 --- a/github/MainClass.py +++ b/github/MainClass.py @@ -53,7 +53,6 @@ import datetime import pickle import time -import sys import requests import jwt import urllib3 @@ -78,8 +77,6 @@ from . import Invitation from . import Consts import six -atLeastPython3 = sys.hexversion >= 0x03000000 - DEFAULT_BASE_URL = "https://api.github.com" DEFAULT_STATUS_URL = "https://status.github.com" # As of 2018-05-17, Github imposes a 10s limit for completion of API requests. @@ -731,7 +728,7 @@ class GithubIntegration(object): algorithm="RS256" ) - if atLeastPython3: + if isinstance(encrypted, bytes): encrypted = encrypted.decode('utf-8') return encrypted diff --git a/github/Repository.py b/github/Repository.py index 2ae5e63e..84558a1a 100644 --- a/github/Repository.py +++ b/github/Repository.py @@ -86,10 +86,10 @@ ################################################################################ from __future__ import absolute_import -import sys import six.moves.urllib.parse import datetime from base64 import b64encode +import collections import github.GithubObject import github.PaginatedList @@ -137,8 +137,6 @@ import github.View from . import Consts import six -atLeastPython3 = sys.hexversion >= 0x03000000 - class Repository(github.GithubObject.CompletableGithubObject): """ @@ -1647,14 +1645,9 @@ class Repository(github.GithubObject.CompletableGithubObject): or isinstance(committer, github.InputGitAuthor), \ 'committer must be a github.InputGitAuthor object' - if atLeastPython3: - if isinstance(content, str): - content = content.encode('utf-8') - content = b64encode(content).decode('utf-8') - else: - if isinstance(content, six.text_type): - content = content.encode('utf-8') - content = b64encode(content) + content = b64encode(bytearray(content, 'utf-8')) + if isinstance(content, bytes): + content = content.decode('utf-8') put_parameters = {'message': message, 'content': content} if branch is not github.GithubObject.NotSet: @@ -1709,14 +1702,9 @@ class Repository(github.GithubObject.CompletableGithubObject): or isinstance(committer, github.InputGitAuthor), \ 'committer must be a github.InputGitAuthor object' - if atLeastPython3: - if isinstance(content, str): - content = content.encode('utf-8') - content = b64encode(content).decode('utf-8') - else: - if isinstance(content, six.text_type): - content = content.encode('utf-8') - content = b64encode(content) + content = b64encode(bytearray(content, 'utf-8')) + if isinstance(content, bytes): + content = content.decode('utf-8') put_parameters = {'message': message, 'content': content, 'sha': sha} @@ -2789,11 +2777,10 @@ class Repository(github.GithubObject.CompletableGithubObject): assert isinstance(callback, (str, six.text_type)), callback assert secret is github.GithubObject.NotSet or isinstance(secret, (str, six.text_type)), secret - post_parameters = { - "hub.mode": mode, - "hub.topic": "https://github.com/" + self.full_name + "/events/" + event, - "hub.callback": callback, - } + post_parameters = collections.OrderedDict() + post_parameters["hub.callback"] = callback + post_parameters["hub.topic"] = "https://github.com/" + self.full_name + "/events/" + event + post_parameters["hub.mode"] = mode if secret is not github.GithubObject.NotSet: post_parameters["hub.secret"] = secret diff --git a/github/Requester.py b/github/Requester.py index cb52252c..f3152b7f 100644 --- a/github/Requester.py +++ b/github/Requester.py @@ -60,7 +60,6 @@ import mimetypes import os import re import requests -import sys import time import six.moves.urllib.parse from io import IOBase @@ -69,8 +68,6 @@ from . import Consts from . import GithubException import six -atLeastPython3 = sys.hexversion >= 0x03000000 - class RequestsResponse: # mimic the httplib response object @@ -458,8 +455,6 @@ class Requester: def __createConnection(self): kwds = {} - if not atLeastPython3: # pragma no branch (Branch useful only with Python 3) - kwds["strict"] = True # Useless in Python3, would generate a deprecation warning kwds["timeout"] = self.__timeout kwds["verify"] = self.__verify diff --git a/tests/Exceptions.py b/tests/Exceptions.py index 07fe936a..0f137765 100644 --- a/tests/Exceptions.py +++ b/tests/Exceptions.py @@ -33,14 +33,11 @@ from __future__ import absolute_import import github -import sys import pickle from . import Framework from six.moves import range -atMostPython2 = sys.hexversion < 0x03000000 - class Exceptions(Framework.TestCase): def testInvalidInput(self): @@ -78,30 +75,21 @@ class Exceptions(Framework.TestCase): self.g.get_user().get_repo("Xxx") self.assertEqual(raisedexp.exception.status, 404) self.assertEqual(raisedexp.exception.data, {"message": "Not Found"}) - if atMostPython2: - self.assertEqual(str(raisedexp.exception), "404 {u'message': u'Not Found'}") - else: - self.assertEqual(str(raisedexp.exception), "404 {'message': 'Not Found'}") # pragma no cover (Covered with Python 3) + self.assertEqual(str(raisedexp.exception), '404 {"message": "Not Found"}') def testUnknownUser(self): with self.assertRaises(github.GithubException) as raisedexp: self.g.get_user("ThisUserShouldReallyNotExist") self.assertEqual(raisedexp.exception.status, 404) self.assertEqual(raisedexp.exception.data, {"message": "Not Found"}) - if atMostPython2: - self.assertEqual(str(raisedexp.exception), "404 {u'message': u'Not Found'}") - else: - self.assertEqual(str(raisedexp.exception), "404 {'message': 'Not Found'}") # pragma no cover (Covered with Python 3) + self.assertEqual(str(raisedexp.exception), '404 {"message": "Not Found"}') def testBadAuthentication(self): with self.assertRaises(github.GithubException) as raisedexp: github.Github("BadUser", "BadPassword").get_user().login self.assertEqual(raisedexp.exception.status, 401) self.assertEqual(raisedexp.exception.data, {"message": "Bad credentials"}) - if atMostPython2: - self.assertEqual(str(raisedexp.exception), "401 {u'message': u'Bad credentials'}") - else: - self.assertEqual(str(raisedexp.exception), "401 {'message': 'Bad credentials'}") # pragma no cover (Covered with Python 3) + self.assertEqual(str(raisedexp.exception), '401 {"message": "Bad credentials"}') def testExceptionPickling(self): pickle.loads(pickle.dumps(github.GithubException('foo', 'bar'))) diff --git a/tests/Framework.py b/tests/Framework.py index 8e19f267..c81d3eb9 100644 --- a/tests/Framework.py +++ b/tests/Framework.py @@ -40,7 +40,6 @@ from __future__ import absolute_import from __future__ import print_function import json import os -import sys import traceback import unittest import httpretty @@ -50,15 +49,12 @@ from urllib3.util import Url import github import six -python2 = sys.hexversion < 0x03000000 -atLeastPython3 = sys.hexversion >= 0x03000000 - -def readLine(file): - if atLeastPython3: - return file.readline().decode("utf-8").strip() - else: - return file.readline().strip() +def readLine(file_): + line = file_.readline() + if isinstance(line, bytes): + line = line.decode('utf-8') + return line.strip() class FakeHttpResponse: @@ -128,10 +124,9 @@ class RecordingConnection: # pragma no cover (Class useful only when recording self.__writeLine(str(status)) self.__writeLine(str(list(headers))) - if atLeastPython3: # In Py3, return from "read" is bytes - self.__writeLine(output) - else: - self.__writeLine(output.encode("utf-8")) + if isinstance(output, bytes): + output = output.decode('utf-8') + self.__writeLine(output) return FakeHttpResponse(status, headers, output) @@ -140,13 +135,9 @@ class RecordingConnection: # pragma no cover (Class useful only when recording return self.__cnx.close() def __writeLine(self, line): - if atLeastPython3: - try: # Detect str/bytes - self.__file.write(line + b"\n") - except TypeError: - self.__file.write((line + "\n").encode('utf-8')) - else: - self.__file.write(line + "\n") + if isinstance(line, bytes): + line = line.decode('utf-8') + self.__file.write(line + '\n') class RecordingHttpConnection(RecordingConnection): # pragma no cover (Class useful only when recording new tests, not used during automated tests) @@ -197,9 +188,7 @@ class ReplayingConnection: if isinstance(input, (str, six.text_type)): if input.startswith("{"): self.__testCase.assertEqual(json.loads(input.replace('\n', '').replace('\r', '')), json.loads(expectedInput)) - elif python2: # @todo Test in all cases, including Python 3.4+ - # In Python 3.4+, dicts are not output in the same order as in Python 2.7. - # So, form-data encoding is not deterministic and is difficult to test. + else: self.__testCase.assertEqual(input.replace('\n', '').replace('\r', ''), expectedInput) else: # for non-string input (e.g. upload asset), let it pass. @@ -218,12 +207,9 @@ class ReplayingConnection: status = int(readLine(self.__file)) self.response_headers = CaseInsensitiveDict(eval(readLine(self.__file))) - output = readLine(self.__file) + output = bytearray(readLine(self.__file), 'utf-8') readLine(self.__file) - if atLeastPython3: - output = bytes(output, 'utf-8') - # make a copy of the headers and remove the ones that interfere with the response handling adding_headers = CaseInsensitiveDict(self.response_headers) adding_headers.pop('content-length', None) diff --git a/tests/Issue494.py b/tests/Issue494.py index 0d59087d..50337bd5 100644 --- a/tests/Issue494.py +++ b/tests/Issue494.py @@ -34,6 +34,6 @@ class Issue494(Framework.TestCase): self.pull = self.repo.get_pull(465) def testRepr(self): - expected = 'PullRequest(title="Change SetHostnameCustomizer to check if ' \ - '/etc/sysconfig/network exist…", number=465)' + expected = u'PullRequest(title="Change SetHostnameCustomizer to check if ' \ + u'/etc/sysconfig/network exist…", number=465)' self.assertEqual(self.pull.__repr__(), expected) diff --git a/tests/Search.py b/tests/Search.py index 31553fe3..c7b0d1ca 100644 --- a/tests/Search.py +++ b/tests/Search.py @@ -28,9 +28,6 @@ from __future__ import absolute_import from . import Framework -import sys - -atLeastPython3 = sys.hexversion >= 0x03000000 class Search(Framework.TestCase): @@ -78,10 +75,10 @@ class Search(Framework.TestCase): files = self.g.search_code("toto", sort="indexed", order="asc", user="jacquev6") self.assertListKeyEqual(files, lambda f: f.name, [u'Commit.setUp.txt', u'PullRequest.testGetFiles.txt', u'NamedUser.testGetEvents.txt', u'PullRequest.testCreateComment.txt', u'PullRequestFile.setUp.txt', u'Repository.testGetIssuesWithWildcards.txt', u'Repository.testGetIssuesWithArguments.txt', u'test_ebnf.cpp', u'test_abnf.cpp', u'PullRequestFile.py', u'SystemCalls.py', u'tests.py', u'LexerTestCase.py', u'ParserTestCase.py']) self.assertEqual(files[0].repository.full_name, "jacquev6/PyGithub") - if atLeastPython3: - self.assertEqual(files[0].decoded_content[:30], b'https\nGET\napi.github.com\nNone\n') - else: - self.assertEqual(files[0].decoded_content[:30], "https\nGET\napi.github.com\nNone\n") + content = files[0].decoded_content + if isinstance(content, bytes): + content = content.decode('utf-8') + self.assertEqual(content[:30], 'https\nGET\napi.github.com\nNone\n') def testSearchHighlightingCode(self): files = self.g.search_code("toto", sort="indexed", order="asc", user="jacquev6", highlight=True)