From a13ce33beb005802fb433724db3ef959259e9b6b Mon Sep 17 00:00:00 2001 From: Vincent Jacques Date: Tue, 25 Sep 2012 22:56:43 +0200 Subject: [PATCH] Improve logging (related to #88) --- github/Logging.py | 5 --- github/Requester.py | 32 ++++++++++++------- github/__init__.py | 8 ++++- github/tests/Logging.py | 30 ++++++++++++----- .../Logging.testLoggingWithBaseUrl.txt | 5 +++ ...ng.testLoggingWithBasicAuthentication.txt} | 0 ...ing.testLoggingWithOAuthAuthentication.txt | 5 +++ ...gging.testLoggingWithoutAuthentication.txt | 5 +++ 8 files changed, 64 insertions(+), 26 deletions(-) delete mode 100644 github/Logging.py create mode 100644 github/tests/ReplayData/Logging.testLoggingWithBaseUrl.txt rename github/tests/ReplayData/{Logging.testLogging.txt => Logging.testLoggingWithBasicAuthentication.txt} (100%) create mode 100644 github/tests/ReplayData/Logging.testLoggingWithOAuthAuthentication.txt create mode 100644 github/tests/ReplayData/Logging.testLoggingWithoutAuthentication.txt diff --git a/github/Logging.py b/github/Logging.py deleted file mode 100644 index 284ccba5..00000000 --- a/github/Logging.py +++ /dev/null @@ -1,5 +0,0 @@ -import logging - - -def get_logger(): - return logging.getLogger('github') diff --git a/github/Requester.py b/github/Requester.py index a97ff019..aad22e57 100644 --- a/github/Requester.py +++ b/github/Requester.py @@ -26,7 +26,6 @@ else: # pragma no cover import simplejson as json # pragma no cover import GithubException -import Logging class Requester: @@ -86,12 +85,13 @@ class Requester: url = o.path if o.query != "": url += "?" + o.query + url = self.__completeUrl(url, parameters) - headers = dict() + requestHeaders = dict() if input is not None: - headers["Content-Type"] = "application/json" + requestHeaders["Content-Type"] = "application/json" if self.__authorizationHeader is not None: - headers["Authorization"] = self.__authorizationHeader + requestHeaders["Authorization"] = self.__authorizationHeader if atLeastPython26: cnx = self.__connectionClass(host=self.__hostname, port=self.__port, strict=True, timeout=self.__timeout) @@ -99,25 +99,33 @@ class Requester: cnx = self.__connectionClass(host=self.__hostname, port=self.__port, strict=True) # pragma no cover cnx.request( verb, - self.__completeUrl(url, parameters), + url, json.dumps(input), - headers + requestHeaders ) response = cnx.getresponse() status = response.status - headers = dict(response.getheaders()) + responseHeaders = dict(response.getheaders()) output = response.read() cnx.close() - if "x-ratelimit-remaining" in headers and "x-ratelimit-limit" in headers: - self.rate_limiting = (int(headers["x-ratelimit-remaining"]), int(headers["x-ratelimit-limit"])) + if "x-ratelimit-remaining" in responseHeaders and "x-ratelimit-limit" in responseHeaders: + self.rate_limiting = (int(responseHeaders["x-ratelimit-remaining"]), int(responseHeaders["x-ratelimit-limit"])) - logger = Logging.get_logger() + logger = logging.getLogger(__name__) if logger.isEnabledFor(logging.DEBUG): - logger.debug(' '.join(map(str, [verb, self.__base_url + url, parameters, input, "==>", status, str(headers), str(output)]))) - return status, headers, output + if "Authorization" in requestHeaders: + if requestHeaders["Authorization"].startswith("Basic"): + requestHeaders["Authorization"] = "Basic (login and password removed)" + elif requestHeaders["Authorization"].startswith("token"): + requestHeaders["Authorization"] = "token (oauth token removed)" + else: + requestHeaders["Authorization"] = "Unknown authorization removed" + logger.debug("%s %s://%s%s %s %s ==> %i %s %s", str(verb), self.__scheme, self.__hostname, str(url), str(requestHeaders), str(input), status, str(responseHeaders), str(output)) + + return status, responseHeaders, output def __completeUrl(self, url, parameters): if parameters is None or len(parameters) == 0: diff --git a/github/__init__.py b/github/__init__.py index 91c2954c..02437110 100644 --- a/github/__init__.py +++ b/github/__init__.py @@ -11,9 +11,15 @@ # You should have received a copy of the GNU Lesser General Public License along with PyGithub. If not, see . +import logging + from Github import Github from GithubException import GithubException from InputFileContent import InputFileContent from InputGitAuthor import InputGitAuthor from InputGitTreeElement import InputGitTreeElement -from Logging import get_logger + +def enable_console_debug_logging(): + logger = logging.getLogger("github") + logger.setLevel(logging.DEBUG) + logger.addHandler(logging.StreamHandler()) diff --git a/github/tests/Logging.py b/github/tests/Logging.py index f5ae745a..f0c09e87 100644 --- a/github/tests/Logging.py +++ b/github/tests/Logging.py @@ -18,7 +18,7 @@ import github import Framework -class Logging(Framework.TestCase): +class Logging(Framework.BasicTestCase): class MockHandler: def __init__(self): self.level = logging.DEBUG @@ -27,12 +27,26 @@ class Logging(Framework.TestCase): def handle(self, record): self.handled = record.getMessage() - def testLogging(self): - self.maxDiff = None - logger = github.get_logger() + def setUp( self ): + Framework.BasicTestCase.setUp(self) + logger = logging.getLogger("github") logger.setLevel(logging.DEBUG) - handler = self.MockHandler() - logger.addHandler(handler) + self.__handler = self.MockHandler() + logger.addHandler(self.__handler) - self.assertEqual(self.g.get_user().name, "Vincent Jacques") - self.assertEqual(handler.handled, u'GET https://api.github.com/user None None ==> 200 {\'status\': \'200 OK\', \'content-length\': \'806\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept, Authorization, Cookie\', \'x-ratelimit-remaining\': \'4993\', \'server\': \'nginx\', \'last-modified\': \'Fri, 14 Sep 2012 18:47:46 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"434dfe5d3f50558fe3cea087cb95c401"\', \'cache-control\': \'private, s-maxage=60, max-age=60\', \'date\': \'Mon, 17 Sep 2012 17:12:32 GMT\', \'content-type\': \'application/json; charset=utf-8\'} {"owned_private_repos":3,"disk_usage":18612,"following":28,"type":"User","public_repos":13,"location":"Paris, France","company":"Criteo","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","plan":{"space":614400,"private_repos":5,"name":"micro","collaborators":1},"blog":"http://vincent-jacques.net","login":"jacquev6","public_gists":3,"html_url":"https://github.com/jacquev6","hireable":false,"created_at":"2010-07-09T06:10:06Z","private_gists":5,"followers":13,"name":"Vincent Jacques","email":"vincent@vincent-jacques.net","bio":"","total_private_repos":3,"collaborators":0,"gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"url":"https://api.github.com/users/jacquev6"}') + def testLoggingWithBasicAuthentication(self): + self.assertEqual(github.Github(self.login, self.password).get_user().name, "Vincent Jacques") + self.assertEqual(self.__handler.handled, u'GET https://api.github.com/user {\'Authorization\': \'Basic (login and password removed)\'} None ==> 200 {\'status\': \'200 OK\', \'content-length\': \'806\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept, Authorization, Cookie\', \'x-ratelimit-remaining\': \'4993\', \'server\': \'nginx\', \'last-modified\': \'Fri, 14 Sep 2012 18:47:46 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"434dfe5d3f50558fe3cea087cb95c401"\', \'cache-control\': \'private, s-maxage=60, max-age=60\', \'date\': \'Mon, 17 Sep 2012 17:12:32 GMT\', \'content-type\': \'application/json; charset=utf-8\'} {"owned_private_repos":3,"disk_usage":18612,"following":28,"type":"User","public_repos":13,"location":"Paris, France","company":"Criteo","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","plan":{"space":614400,"private_repos":5,"name":"micro","collaborators":1},"blog":"http://vincent-jacques.net","login":"jacquev6","public_gists":3,"html_url":"https://github.com/jacquev6","hireable":false,"created_at":"2010-07-09T06:10:06Z","private_gists":5,"followers":13,"name":"Vincent Jacques","email":"vincent@vincent-jacques.net","bio":"","total_private_repos":3,"collaborators":0,"gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"url":"https://api.github.com/users/jacquev6"}') + + def testLoggingWithOAuthAuthentication(self): + self.assertEqual(github.Github(self.oauth_token).get_user().name, "Vincent Jacques") + self.assertEqual(self.__handler.handled, u'GET https://api.github.com/user {\'Authorization\': \'token (oauth token removed)\'} None ==> 200 {\'status\': \'200 OK\', \'x-ratelimit-remaining\': \'4993\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept, Authorization, Cookie\', \'content-length\': \'628\', \'server\': \'nginx\', \'last-modified\': \'Tue, 25 Sep 2012 07:42:42 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"c23ad6b5815fc3d6ec6341c4a47afe85"\', \'cache-control\': \'private, max-age=60, s-maxage=60\', \'date\': \'Tue, 25 Sep 2012 20:36:54 GMT\', \'x-oauth-scopes\': \'\', \'content-type\': \'application/json; charset=utf-8\', \'x-accepted-oauth-scopes\': \'user\'} {"type":"User","bio":"","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","blog":"http://vincent-jacques.net","public_repos":13,"created_at":"2010-07-09T06:10:06Z","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","email":"vincent@vincent-jacques.net","following":29,"name":"Vincent Jacques","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","hireable":false,"id":327146,"public_gists":3,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}') + + def testLoggingWithoutAuthentication(self): + self.assertEqual(github.Github().get_user("jacquev6").name, "Vincent Jacques") + self.assertEqual(self.__handler.handled, u'GET https://api.github.com/users/jacquev6 {} None ==> 200 {\'status\': \'200 OK\', \'content-length\': \'628\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept\', \'x-ratelimit-remaining\': \'4989\', \'server\': \'nginx\', \'last-modified\': \'Tue, 25 Sep 2012 07:42:42 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"9bd085221a16b6d2ea95e72634c3c1ac"\', \'cache-control\': \'public, max-age=60, s-maxage=60\', \'date\': \'Tue, 25 Sep 2012 20:38:56 GMT\', \'content-type\': \'application/json; charset=utf-8\'} {"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}') + + def testLoggingWithBaseUrl(self): + # ReplayData forged, not recorded + self.assertEqual(github.Github(base_url = "http://my.enterprise.com/my/prefix").get_user("jacquev6").name, "Vincent Jacques") + self.assertEqual(self.__handler.handled, u'GET http://my.enterprise.com/my/prefix/users/jacquev6 {} None ==> 200 {\'status\': \'200 OK\', \'content-length\': \'628\', \'x-github-media-type\': \'github.beta; format=json\', \'x-content-type-options\': \'nosniff\', \'vary\': \'Accept\', \'x-ratelimit-remaining\': \'4989\', \'server\': \'nginx\', \'last-modified\': \'Tue, 25 Sep 2012 07:42:42 GMT\', \'connection\': \'keep-alive\', \'x-ratelimit-limit\': \'5000\', \'etag\': \'"9bd085221a16b6d2ea95e72634c3c1ac"\', \'cache-control\': \'public, max-age=60, s-maxage=60\', \'date\': \'Tue, 25 Sep 2012 20:38:56 GMT\', \'content-type\': \'application/json; charset=utf-8\'} {"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"}') diff --git a/github/tests/ReplayData/Logging.testLoggingWithBaseUrl.txt b/github/tests/ReplayData/Logging.testLoggingWithBaseUrl.txt new file mode 100644 index 00000000..863d0aad --- /dev/null +++ b/github/tests/ReplayData/Logging.testLoggingWithBaseUrl.txt @@ -0,0 +1,5 @@ +http GET my.enterprise.com None /my/prefix/users/jacquev6 {} null +200 +[('status', '200 OK'), ('content-length', '628'), ('x-github-media-type', 'github.beta; format=json'), ('x-content-type-options', 'nosniff'), ('x-ratelimit-limit', '5000'), ('vary', 'Accept'), ('x-ratelimit-remaining', '4989'), ('server', 'nginx'), ('last-modified', 'Tue, 25 Sep 2012 07:42:42 GMT'), ('connection', 'keep-alive'), ('etag', '"9bd085221a16b6d2ea95e72634c3c1ac"'), ('cache-control', 'public, max-age=60, s-maxage=60'), ('date', 'Tue, 25 Sep 2012 20:38:56 GMT'), ('content-type', 'application/json; charset=utf-8')] +{"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"} + diff --git a/github/tests/ReplayData/Logging.testLogging.txt b/github/tests/ReplayData/Logging.testLoggingWithBasicAuthentication.txt similarity index 100% rename from github/tests/ReplayData/Logging.testLogging.txt rename to github/tests/ReplayData/Logging.testLoggingWithBasicAuthentication.txt diff --git a/github/tests/ReplayData/Logging.testLoggingWithOAuthAuthentication.txt b/github/tests/ReplayData/Logging.testLoggingWithOAuthAuthentication.txt new file mode 100644 index 00000000..4a205729 --- /dev/null +++ b/github/tests/ReplayData/Logging.testLoggingWithOAuthAuthentication.txt @@ -0,0 +1,5 @@ +https GET api.github.com None /user {'Authorization': 'token private_token_removed'} null +200 +[('status', '200 OK'), ('x-ratelimit-remaining', '4993'), ('x-github-media-type', 'github.beta; format=json'), ('x-content-type-options', 'nosniff'), ('vary', 'Accept, Authorization, Cookie'), ('content-length', '628'), ('server', 'nginx'), ('last-modified', 'Tue, 25 Sep 2012 07:42:42 GMT'), ('connection', 'keep-alive'), ('x-ratelimit-limit', '5000'), ('etag', '"c23ad6b5815fc3d6ec6341c4a47afe85"'), ('cache-control', 'private, max-age=60, s-maxage=60'), ('date', 'Tue, 25 Sep 2012 20:36:54 GMT'), ('x-oauth-scopes', ''), ('content-type', 'application/json; charset=utf-8'), ('x-accepted-oauth-scopes', 'user')] +{"type":"User","bio":"","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","blog":"http://vincent-jacques.net","public_repos":13,"created_at":"2010-07-09T06:10:06Z","avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","email":"vincent@vincent-jacques.net","following":29,"name":"Vincent Jacques","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","hireable":false,"id":327146,"public_gists":3,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"} + diff --git a/github/tests/ReplayData/Logging.testLoggingWithoutAuthentication.txt b/github/tests/ReplayData/Logging.testLoggingWithoutAuthentication.txt new file mode 100644 index 00000000..3d991096 --- /dev/null +++ b/github/tests/ReplayData/Logging.testLoggingWithoutAuthentication.txt @@ -0,0 +1,5 @@ +https GET api.github.com None /users/jacquev6 {} null +200 +[('status', '200 OK'), ('content-length', '628'), ('x-github-media-type', 'github.beta; format=json'), ('x-content-type-options', 'nosniff'), ('x-ratelimit-limit', '5000'), ('vary', 'Accept'), ('x-ratelimit-remaining', '4989'), ('server', 'nginx'), ('last-modified', 'Tue, 25 Sep 2012 07:42:42 GMT'), ('connection', 'keep-alive'), ('etag', '"9bd085221a16b6d2ea95e72634c3c1ac"'), ('cache-control', 'public, max-age=60, s-maxage=60'), ('date', 'Tue, 25 Sep 2012 20:38:56 GMT'), ('content-type', 'application/json; charset=utf-8')] +{"type":"User","html_url":"https://github.com/jacquev6","login":"jacquev6","followers":14,"company":"Criteo","created_at":"2010-07-09T06:10:06Z","email":"vincent@vincent-jacques.net","hireable":false,"avatar_url":"https://secure.gravatar.com/avatar/b68de5ae38616c296fa345d2b9df2225?d=https://a248.e.akamai.net/assets.github.com%2Fimages%2Fgravatars%2Fgravatar-user-420.png","public_gists":3,"bio":"","following":29,"name":"Vincent Jacques","blog":"http://vincent-jacques.net","gravatar_id":"b68de5ae38616c296fa345d2b9df2225","id":327146,"public_repos":13,"location":"Paris, France","url":"https://api.github.com/users/jacquev6"} +