From 29d4f149aaff02ef160aecf3268ce94a054d8fca Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 16:22:09 -0400 Subject: [PATCH 01/18] Version bump: v2.0 --- agithub.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agithub.py b/agithub.py index 5db3c6f..c3ef2d5 100644 --- a/agithub.py +++ b/agithub.py @@ -15,7 +15,7 @@ import urllib as urllib urllib.parse = urllib -VERSION = [1,2] +VERSION = [2,0] STR_VERSION = 'v' + '.'.join(str(v) for v in VERSION) # These headers are implicitly included in each request; however, each From 996e6105df8dd11207baf0a0143ab37c511ce1aa Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 14:51:09 -0400 Subject: [PATCH 02/18] Style: under_score -> camelCase conversion This is, of course, very subjective, but the rule currently is: * Most things are camelCase * Constants are capatalized and under_scored * Function/Method named parameters are under_scored * Members of the ConnectionProperies class are under_scored * The Content "media-type handlers" still use under_scores This is a technical limitation, and isn't likely to change. It does (helpfully) emphasize them as "special." --- agithub.py | 80 +++++++++++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/agithub.py b/agithub.py index c3ef2d5..5277aa3 100644 --- a/agithub.py +++ b/agithub.py @@ -21,7 +21,7 @@ # These headers are implicitly included in each request; however, each # can be explicitly overridden by the client code. (Used in Client # objects.) -_default_headers = { +DEFAULT_HEADERS = { #XXX: Header field names MUST be lowercase; this is not checked 'user-agent': 'agithub/' + STR_VERSION } @@ -118,7 +118,7 @@ def __init__(self, client): self.url = '' def __getattr__(self, key): - if key in self.client.http_methods: + if key in self.client.httpMethods: mfun = getattr(self.client, key) fun = partial(mfun, url=self.url) return update_wrapper(fun, mfun) @@ -138,7 +138,7 @@ def __repr__(self): return '%s: %s' % (self.__class__, self.url) class Client(object): - http_methods = ( + httpMethods = ( 'head', 'get', 'post', @@ -147,7 +147,7 @@ class Client(object): 'patch', ) - default_headers = {} + defaultHeaders = {} headers = None def __init__(self, username=None, @@ -167,11 +167,11 @@ def __init__(self, username=None, if password is not None and token is not None: raise TypeError("You cannot use both password and oauth token authenication") - self.auth_header = None + self.authHeader = None if password is not None: - self.auth_header = self.hash_pass(password) + self.authHeader = self.hashPassword(password) elif token is not None: - self.auth_header = 'Token %s' % token + self.authHeader = 'Token %s' % token def setConnectionProperties(self, props): ''' @@ -184,33 +184,33 @@ def setConnectionProperties(self, props): self.prop = props if self.prop.extra_headers is not None: - self.default_headers = _default_headers.copy() - self.default_headers.update(self.prop.extra_headers) + self.defaultHeaders = DEFAULT_HEADERS.copy() + self.defaultHeaders.update(self.prop.extra_headers) - # Enforce case restrictions on self.default_headers - tmp_dict = {} - for k,v in self.default_headers.items(): - tmp_dict[k.lower()] = v - self.default_headers = tmp_dict + # Enforce case restrictions on self.defaultHeaders + tmpDict = {} + for k,v in self.defaultHeaders.items(): + tmpDict[k.lower()] = v + self.defaultHeaders = tmpDict def head(self, url, headers={}, **params): - url += self.urlencode(params) + url += self.urlEncode(params) return self.request('HEAD', url, None, headers) def get(self, url, headers={}, **params): - url += self.urlencode(params) + url += self.urlEncode(params) return self.request('GET', url, None, headers) def post(self, url, body=None, headers={}, **params): - url += self.urlencode(params) + url += self.urlEncode(params) return self.request('POST', url, json.dumps(body), headers) def put(self, url, body=None, headers={}, **params): - url += self.urlencode(params) + url += self.urlEncode(params) return self.request('PUT', url, json.dumps(body), headers) def delete(self, url, headers={}, **params): - url += self.urlencode(params) + url += self.urlEncode(params) return self.request('DELETE', url, None, headers) def patch(self, url, body=None, headers={}, **params): @@ -218,19 +218,19 @@ def patch(self, url, body=None, headers={}, **params): Do a http patch request on the given url with given body, headers and parameters Parameters is a dictionary that will will be urlencoded """ - url += self.urlencode(params) + url += self.urlEncode(params) return self.request(self.PATCH, url, json.dumps(body), headers) def request(self, method, url, body, headers): '''Low-level networking. All HTTP-method methods call this''' - headers = self._fix_headers(headers) + headers = self.caseConvertHeaders(headers) if self.username: - headers['authorization'] = self.auth_header + headers['authorization'] = self.authHeader #TODO: Context manager - conn = self.get_connection() + conn = self.getConnection() conn.request(method, url, body, headers) response = conn.getresponse() status = response.status @@ -240,29 +240,29 @@ def request(self, method, url, body, headers): conn.close() return status, content.processBody() - def _fix_headers(self, headers): + def caseConvertHeaders(self, headers): # Convert header names to a uniform case - tmp_dict = {} + tmpDict = {} for k,v in headers.items(): - tmp_dict[k.lower()] = v - headers = tmp_dict + tmpDict[k.lower()] = v + headers = tmpDict # Add default headers (if unspecified) - for k,v in self.default_headers.items(): + for k,v in self.defaultHeaders.items(): if k not in headers: headers[k] = v return headers - def urlencode(self, params): + def urlEncode(self, params): if not params: return '' - return '?' + urllib.parse.urlencode(params) + return '?' + urllib.parse.urlEncode(params) - def hash_pass(self, password): - auth_str = ('%s:%s' % (self.username, password)).encode('utf-8') - return 'Basic '.encode('utf-8') + base64.b64encode(auth_str).strip() + def hashPassword(self, password): + authStr = ('%s:%s' % (self.username, password)).encode('utf-8') + return 'Basic '.encode('utf-8') + base64.b64encode(authStr).strip() - def get_connection(self): + def getConnection(self): if self.prop.secure_http: conn = http.client.HTTPSConnection(self.prop.api_url) elif self.username is None: @@ -282,9 +282,9 @@ class Content(object): def __init__(self, response): self.response = response self.body = response.read() - (self.mediatype, self.encoding) = self.get_ctype() + (self.mediatype, self.encoding) = self.getContentType() - def get_ctype(self): + def getContentType(self): '''Split the content-type field into mediatype and charset''' ctype = self.response.getheader('Content-Type') @@ -305,7 +305,7 @@ def get_ctype(self): return (mediatype, charset) - def decode_body(self): + def decodeBody(self): ''' Decode (and replace) self.body via the charset encoding specified in the content-type header @@ -318,12 +318,12 @@ def processBody(self): Retrieve the body of the response, encoding it into a usuable form based on the media-type (mime-type) ''' - handlerName = self.mangled_mtype() + handlerName = self.funMangledMediaType() handler = getattr(self, handlerName, self.x_application_unknown) return handler() - def mangled_mtype(self): + def funMangledMediaType(self): ''' Mangle the media type into a suitable function name ''' @@ -338,7 +338,7 @@ def x_application_unknown(self): def application_json(self): '''Handler for application/json media-type''' - self.decode_body() + self.decodeBody() try: pybody = json.loads(self.body) From 6c77aabe125df177099147b53046e6f874408d5e Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 15:20:51 -0400 Subject: [PATCH 03/18] Rename class Github -> GitHub, and update documentation to match Now, we spell it the same way as the official GitHub branding. - test.py has also been updated to match --- README.md | 16 ++++++++-------- agithub.py | 16 ++++++++-------- test.py | 6 +++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 991bd2c..83d2b87 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,10 @@ -# The Agnostic Github API +# The Agnostic GitHub API *It doesn't know, and you don't care!* `agithub` is a REST API client tailored to https://api.github.com, with a transparent syntax which facilitates rapid prototyping. It's code is lightweight: easy to understand, modify, and integrate. It's most -salient feature is that it doesn't know the Github API — but +salient feature is that it doesn't know the GitHub API — but that doesn't matter, since it fully supports it *anyway*. While browsing the @@ -28,12 +28,12 @@ can read the docs and immediately know how to do the examples via ## Example App -1. First, instantiate a `Github` object, passing it your username and +1. First, instantiate a `GitHub` object, passing it your username and password if an authenticated session is desired. ```python - >>> from agithub import Github - >>> g = Github('user', 'pass') + >>> from agithub import GitHub + >>> g = GitHub('user', 'pass') ``` 2. When you make a request, the status and response body are passed back @@ -83,7 +83,7 @@ can read the docs and immediately know how to do the examples via You may find this useful — or not. -6. Finally, `agithub` knows nothing at all about the Github API, and it +6. Finally, `agithub` knows nothing at all about the GitHub API, and it won't second-guess you. ```python @@ -91,7 +91,7 @@ can read the docs and immediately know how to do the examples via (404, {'message': 'Not Found'}) ``` - The error message you get is directly from Github's API. This gives + The error message you get is directly from GitHub's API. This gives you all of the information you need to survey the situation. 7. If you need more information, the response headers of the previous @@ -113,7 +113,7 @@ crop up: 1. Networking Exceptions (from the `http` library). Catch these with `try .. catch` blocks, as you otherwise would. -2. Github API errors. These means you're doing something wrong with the +2. GitHub API errors. These means you're doing something wrong with the API, and they are always evident in the response's status. The API considerately returns a helpful error message in the JSON body. diff --git a/agithub.py b/agithub.py index 5277aa3..2f0e2f7 100644 --- a/agithub.py +++ b/agithub.py @@ -31,12 +31,12 @@ class API(object): The toplevel object, and the "entry-point" into the client API. Subclass this to develop an application for a particular REST API. - Model your __init__ after the Github example. + Model your __init__ after the GitHub example. ''' def __init__(self, *args, **kwargs): raise Exception ( 'Please subclass API and override __init__() to' - 'provide a ConnectionProperties object. See the Github' + 'provide a ConnectionProperties object. See the GitHub' ' class for an example' ) @@ -56,10 +56,10 @@ def __repr__(self): def getheaders(self): return self.client.headers -class Github(API): - '''The agnostic Github API. It doesn't know, and you don't care. - >>> from agithub import Github - >>> g = Github('user', 'pass') +class GitHub(API): + '''The agnostic GitHub API. It doesn't know, and you don't care. + >>> from agithub import GitHub + >>> g = GitHub('user', 'pass') >>> status, data = g.issues.get(filter='subscribed') >>> data ... [ list_, of, stuff ] @@ -78,7 +78,7 @@ class Github(API): That's all there is to it. (blah.post() should work, too.) - NOTE: It is up to you to spell things correctly. A Github object + NOTE: It is up to you to spell things correctly. A GitHub object doesn't even try to validate the url you feed it. On the other hand, it automatically supports the full API--so why should you care? ''' @@ -109,7 +109,7 @@ class RequestBuilder(object): You can use item access instead of attribute access. This is convenient for using variables\' values and required for numbers. - >>> Github('user','pass').whatever[1][x][y].post() + >>> GitHub('user','pass').whatever[1][x][y].post() To understand the method(...) calls, check out github.client.Client. ''' diff --git a/test.py b/test.py index 8ef14b6..60d7377 100644 --- a/test.py +++ b/test.py @@ -1,5 +1,5 @@ from __future__ import print_function -from agithub import Github +from agithub import GitHub ## # Test harness @@ -136,7 +136,7 @@ def yesno(ans): ### if __name__ == '__main__': - anonSession = initAnonymousSession(Github) + anonSession = initAnonymousSession(GitHub) authSession = None ans = input( @@ -148,7 +148,7 @@ def yesno(ans): username = input('Username: ') password = input ('Password (plain text): ') authSession = initAuthenticatedSession( - Github + GitHub , username=username, password=password ) From 54b4b08d49c0e472b9a51f308a1ea0309d3bb691 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 15:23:25 -0400 Subject: [PATCH 04/18] agithub:Spellcheck errror diagnostic and docstrings --- agithub.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/agithub.py b/agithub.py index 2f0e2f7..e114276 100644 --- a/agithub.py +++ b/agithub.py @@ -165,7 +165,7 @@ def __init__(self, username=None, if password is None and token is None: raise TypeError("You need a password to authenticate as " + username) if password is not None and token is not None: - raise TypeError("You cannot use both password and oauth token authenication") + raise TypeError("You cannot use both password and OAuth token authentication") self.authHeader = None if password is not None: @@ -216,7 +216,7 @@ def delete(self, url, headers={}, **params): def patch(self, url, body=None, headers={}, **params): """ Do a http patch request on the given url with given body, headers and parameters - Parameters is a dictionary that will will be urlencoded + Parameters is a dictionary that will will be url-encoded """ url += self.urlEncode(params) return self.request(self.PATCH, url, json.dumps(body), headers) @@ -285,7 +285,7 @@ def __init__(self, response): (self.mediatype, self.encoding) = self.getContentType() def getContentType(self): - '''Split the content-type field into mediatype and charset''' + '''Split the content-type field into media-type and char-set''' ctype = self.response.getheader('Content-Type') start = 0 From 079976b5aca01747aa79e2fd3a3d1f915d183f7e Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 16:04:49 -0400 Subject: [PATCH 05/18] Remove duplicate code In the Client class: setConnectionProperties() - Call caseConvertHeaders() instead of duplicating its code caseConvertHeaders() - Only do case conversion New function updateWithDefaultHeaders() - Call caseConvertHeaders() on the input - Do the defaultHeaders update which we removed from caseConvertHeaders() request() - Call updateWithDefaultHeaders() instead of caseConvertHeaders() --- agithub.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/agithub.py b/agithub.py index e114276..efc56c8 100644 --- a/agithub.py +++ b/agithub.py @@ -22,7 +22,6 @@ # can be explicitly overridden by the client code. (Used in Client # objects.) DEFAULT_HEADERS = { - #XXX: Header field names MUST be lowercase; this is not checked 'user-agent': 'agithub/' + STR_VERSION } @@ -188,10 +187,7 @@ def setConnectionProperties(self, props): self.defaultHeaders.update(self.prop.extra_headers) # Enforce case restrictions on self.defaultHeaders - tmpDict = {} - for k,v in self.defaultHeaders.items(): - tmpDict[k.lower()] = v - self.defaultHeaders = tmpDict + self.defaultHeaders = self.caseConvertHeaders(self.defaultHeaders) def head(self, url, headers={}, **params): url += self.urlEncode(params) @@ -224,7 +220,7 @@ def patch(self, url, body=None, headers={}, **params): def request(self, method, url, body, headers): '''Low-level networking. All HTTP-method methods call this''' - headers = self.caseConvertHeaders(headers) + headers = self.updateWithDefaultHeaders(headers) if self.username: headers['authorization'] = self.authHeader @@ -245,9 +241,11 @@ def caseConvertHeaders(self, headers): tmpDict = {} for k,v in headers.items(): tmpDict[k.lower()] = v - headers = tmpDict + return tmpDict - # Add default headers (if unspecified) + def updateWithDefaultHeaders(self, headers): + # Add default headers (if absent) + headers = self.caseConvertHeaders(headers) for k,v in self.defaultHeaders.items(): if k not in headers: headers[k] = v From 90da9d24a105f42e945be2d6b1d2bf176d197ee2 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 16:20:08 -0400 Subject: [PATCH 06/18] TODO: Reschedule "request body encoding" to v2.0 milestone --- ChangeLog.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 3656f93..543df34 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -23,18 +23,6 @@ Upcoming Unscheduled ----------- -* Support encoding/serialization request bodies, analogous to the - decoding/de-serialization for response bodies which is done in the - Content class - - - This probably means reorganizing the Content class, perhaps adding - another level of structure - - Find a convenient way for the user to specify request-body - content-type. Maybe add a `content_type=` parameter to `put()` et - al? - - - Does GitHub support this? It should. And if so, we should use it - by default * Create a script to pack the basic module and any given set of service-specific classes as one file @@ -88,6 +76,19 @@ v2.0 * Support XML de-serialization. Python has (I think) built-in support for this +* Support encoding/serialization request bodies, analogous to the + decoding/de-serialization for response bodies which is done in the + Content class + + - This probably means reorganizing the Content class, perhaps adding + another level of structure + - Find a convenient way for the user to specify request-body + content-type. Maybe add a `content_type=` parameter to `put()` et + al? + + - Does GitHub support this? It should. And if so, we should use it + by default + v1.1.1 ------ From 0a3f4787ef68d7eb12b25d7708ccecb2f9c41789 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 16:20:50 -0400 Subject: [PATCH 07/18] New TODO for milestone v2.0 --- ChangeLog.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index 543df34..9438ca2 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -89,6 +89,9 @@ v2.0 - Does GitHub support this? It should. And if so, we should use it by default +* Parse Content-Type header more correctly; make a dict of the + parameters available to the media-type handlers + v1.1.1 ------ From 958ceee6255eb4002e7502211a987d925757961f Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 16:25:24 -0400 Subject: [PATCH 08/18] Rename Content -> ResponseBody, et al Since we plant to support request-body's in a parallel way, it only makes sense to name the two classes parallelly. On the outside, this shifts focus from the content-type to the response body the basic concept. - Rename Content -> ResponseBody, et al - Rename Content.processBody -> ResponseBody.proces - Rename Client.request()//content -> resBody --- agithub.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agithub.py b/agithub.py index efc56c8..07af518 100644 --- a/agithub.py +++ b/agithub.py @@ -230,11 +230,11 @@ def request(self, method, url, body, headers): conn.request(method, url, body, headers) response = conn.getresponse() status = response.status - content = Content(response) + resBody = ResponseBody(response) self.headers = response.getheaders() conn.close() - return status, content.processBody() + return status, resBody.process() def caseConvertHeaders(self, headers): # Convert header names to a uniform case @@ -273,7 +273,7 @@ def getConnection(self): return conn -class Content(object): +class ResponseBody(object): ''' Decode a response from the server, respecting the Content-Type field ''' @@ -311,7 +311,7 @@ def decodeBody(self): self.body = self.body.decode(self.encoding) - def processBody(self): + def process(self): ''' Retrieve the body of the response, encoding it into a usuable form based on the media-type (mime-type) From 769790999274779bbb690a1263da163c7ed0dfd3 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 17:21:24 -0400 Subject: [PATCH 09/18] Fix RequestBody RFC Compliance: unknown media-type = application/octet-stream As per RFC-2068, Section 7.2.1, line 2364, we use the official 'application/octet-stream' media-type for unknown entity-bodies, instead of the ad-hoc 'x-application/unknown' media-type. --- agithub.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/agithub.py b/agithub.py index 07af518..50d17e2 100644 --- a/agithub.py +++ b/agithub.py @@ -292,7 +292,7 @@ def getContentType(self): end = ctype.index(';') mediatype = ctype[:end] except: - mediatype = 'x-application/unknown' + mediatype = 'application/octet-stream' try: start = 8 + ctype.index('charset=', end) @@ -317,7 +317,9 @@ def process(self): form based on the media-type (mime-type) ''' handlerName = self.funMangledMediaType() - handler = getattr(self, handlerName, self.x_application_unknown) + handler = getattr( self, handlerName, + self.application_octet_stream + ) return handler() @@ -330,8 +332,11 @@ def funMangledMediaType(self): ## media-type handlers - def x_application_unknown(self): - '''Handler for unknown media-types''' + def application_octet_stream(self): + '''Handler for binary data and unknown media-types. Importantly, + it does absolutely no pre-processing of the body, which means it + will not mess it up. + ''' return self.body def application_json(self): From 28f5a31cc32ff297a481e4b7f0699ff04d253c1a Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 19:07:47 -0400 Subject: [PATCH 10/18] ResponseBody: Proper parsing of Content-Type header We've been ignoring the standard (RFC 2068) for a while now, instead expecting every media-type to include a charset parameter (and only that). Now, we corectly handle multiple, varied media-type parameters, giving our media-type handlers that much more flexibility. getContentType removed, superseded by parseContentType new method parseContentType - Stores self.mediatype - Stores self.ctypeParameters, a dict of the parameters - Sets charset correctly, even if it was implicit, so the handler's don't have to implement any "default logic" for it --- agithub.py | 60 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/agithub.py b/agithub.py index 50d17e2..b60566b 100644 --- a/agithub.py +++ b/agithub.py @@ -280,28 +280,52 @@ class ResponseBody(object): def __init__(self, response): self.response = response self.body = response.read() - (self.mediatype, self.encoding) = self.getContentType() + self.parseContentType(self.response.getheader('Content-Type')) + self.encoding = self.ctypeParameters['charset'] - def getContentType(self): - '''Split the content-type field into media-type and char-set''' - ctype = self.response.getheader('Content-Type') + def parseContentType(self, ctype): + ''' + Parse the Content-Type header, returning the media-type and any + parameters + ''' - start = 0 - end = 0 - try: - end = ctype.index(';') - mediatype = ctype[:end] - except: - mediatype = 'application/octet-stream' + if ctype is None: + self.mediatype = 'application/octet-stream' + self.ctypeParameters = { 'charset': 'ISO-8859-1' } + return + + params = ctype.split(';') + self.mediatype = params.pop(0).strip() + + # Parse parameters + if len(params) > 0: + params = map( lambda s : s.strip().split('=') + , params + ) + + paramDict = {} + for attribute, value in params: + # TODO: Find out if specifying an attribute multiple + # times is even okay, and how it should be handled + attribute = attribute.lower() + if attribute in paramDict: + if type(paramDict[attribute]) is not list: + # Convert singleton value to value-list + paramDict[attribute] = [paramDict[attribute]] + # Insert new value along with pre-existing ones + paramDict[attribute] += value + else: + # Insert singleton attribute value + paramDict[attribute] = value + self.ctypeParameters = paramDict - try: - start = 8 + ctype.index('charset=', end) - end = ctype.index(';', start) - charset = ctype[start:end].rstrip() - except: - charset = 'ISO-8859-1' #TODO + else: + self.ctypeParameters = {} - return (mediatype, charset) + if 'charset' not in self.ctypeParameters: + self.ctypeParameters['charset'] = 'ISO-8859-1' + # NB: ISO-8859-1 is specified (RFC 2068) as the default + # charset in case none is provided def decodeBody(self): ''' From ff6c0e89ab8e38569a5b758ce057a5ba67404288 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 19:28:45 -0400 Subject: [PATCH 11/18] Split ResponseBody into a superclass (Body) and subclass (ResponseBody) Body contains those parts of a response body which are conceptually or algorithmically shared with a request body, such as content-type decoding. This is in preparation for a new RequestBody class. --- agithub.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/agithub.py b/agithub.py index b60566b..23af8f5 100644 --- a/agithub.py +++ b/agithub.py @@ -273,15 +273,8 @@ def getConnection(self): return conn -class ResponseBody(object): - ''' - Decode a response from the server, respecting the Content-Type field - ''' - def __init__(self, response): - self.response = response - self.body = response.read() - self.parseContentType(self.response.getheader('Content-Type')) - self.encoding = self.ctypeParameters['charset'] +class Body(object): + '''Superclass for ResponseBody and RequestBody''' def parseContentType(self, ctype): ''' @@ -327,6 +320,23 @@ def parseContentType(self, ctype): # NB: ISO-8859-1 is specified (RFC 2068) as the default # charset in case none is provided + def funMangledMediaType(self): + ''' + Mangle the media type into a suitable function name + ''' + return self.mediatype.replace('-','_').replace('/','_') + + +class ResponseBody(Body): + ''' + Decode a response from the server, respecting the Content-Type field + ''' + def __init__(self, response): + self.response = response + self.body = response.read() + self.parseContentType(self.response.getheader('Content-Type')) + self.encoding = self.ctypeParameters['charset'] + def decodeBody(self): ''' Decode (and replace) self.body via the charset encoding @@ -347,13 +357,6 @@ def process(self): return handler() - def funMangledMediaType(self): - ''' - Mangle the media type into a suitable function name - ''' - return self.mediatype.replace('-','_').replace('/','_') - - ## media-type handlers def application_octet_stream(self): From f0f09ac3a0f6119b84c7af4ae0cd98664faae553 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 19:46:26 -0400 Subject: [PATCH 12/18] New class RequestBody: Content-Type encoding of the request body Client: - The HTTP methods which accept a body now pass it unchanged to Client.request - Client.request now constructs a RequestBody, and uses it to process the user-supplied body RequestBody Handles charset encoding and media-type serialization; it is structured very similarly to ResponseBody --- agithub.py | 63 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 5 deletions(-) diff --git a/agithub.py b/agithub.py index 23af8f5..22272ce 100644 --- a/agithub.py +++ b/agithub.py @@ -199,11 +199,11 @@ def get(self, url, headers={}, **params): def post(self, url, body=None, headers={}, **params): url += self.urlEncode(params) - return self.request('POST', url, json.dumps(body), headers) + return self.request('POST', url, body, headers) def put(self, url, body=None, headers={}, **params): url += self.urlEncode(params) - return self.request('PUT', url, json.dumps(body), headers) + return self.request('PUT', url, body, headers) def delete(self, url, headers={}, **params): url += self.urlEncode(params) @@ -215,7 +215,7 @@ def patch(self, url, body=None, headers={}, **params): Parameters is a dictionary that will will be url-encoded """ url += self.urlEncode(params) - return self.request(self.PATCH, url, json.dumps(body), headers) + return self.request(self.PATCH, url, body, headers) def request(self, method, url, body, headers): '''Low-level networking. All HTTP-method methods call this''' @@ -225,9 +225,9 @@ def request(self, method, url, body, headers): if self.username: headers['authorization'] = self.authHeader - #TODO: Context manager + reqBody = RequestBody(body, headers) conn = self.getConnection() - conn.request(method, url, body, headers) + conn.request(method, url, reqBody.process(), headers) response = conn.getresponse() status = response.status resBody = ResponseBody(response) @@ -383,6 +383,59 @@ def application_json(self): # Insert new media-type handlers here +class RequestBody(Body): + ''' + Encode a request body from the client, respecting the Content-Type + field + ''' + def __init__(self, body, headers): + self.body = body + self.headers = headers + self.parseContentType( + getattr(self.headers, 'content-type', None) + ) + self.encoding = self.ctypeParameters['charset'] + + def encodeBody(self): + ''' + Encode (and overwrite) self.body via the charset encoding + specified in the request headers + ''' + self.body = self.body.encode(self.encoding) + + def process(self): + ''' + Process the request body by applying a media-type specific + handler to it. Some media-types need charset encoding as well, + and it is up to the handler to do this by calling + self.encodeBody() + ''' + if self.body is None: + return None + + handlerName = self.funMangledMediaType() + handler = getattr( self, handlerName, + self.application_octet_stream + ) + return handler() + + ## media-type handlers + + def application_octet_stream(self): + '''Handler for binary data and unknown media-types. Importantly, + it does absolutely no pre-processing of the body, which means it + will not mess it up. + ''' + return self.body + + def application_json(self): + self.body = json.dumps(self.body) + self.encodeBody() + return self.body + + # Insert new Request media-type handlers here + + class ConnectionProperties(object): __slots__ = ['api_url', 'secure_http', 'extra_headers'] From fad02faaf393c54d23a65b1a0220840669af9011 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 20:37:01 -0400 Subject: [PATCH 13/18] Docstring and comment fixes --- agithub.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/agithub.py b/agithub.py index 22272ce..a2a5a54 100644 --- a/agithub.py +++ b/agithub.py @@ -210,10 +210,11 @@ def delete(self, url, headers={}, **params): return self.request('DELETE', url, None, headers) def patch(self, url, body=None, headers={}, **params): - """ - Do a http patch request on the given url with given body, headers and parameters - Parameters is a dictionary that will will be url-encoded - """ + ''' + Do a http patch request on the given url with given body, + headers and parameters Parameters is a dictionary that will will + be url-encoded + ''' url += self.urlEncode(params) return self.request(self.PATCH, url, body, headers) @@ -344,10 +345,9 @@ def decodeBody(self): ''' self.body = self.body.decode(self.encoding) - def process(self): ''' - Retrieve the body of the response, encoding it into a usuable + Retrieve the body of the response, encoding it into a usable form based on the media-type (mime-type) ''' handlerName = self.funMangledMediaType() @@ -381,7 +381,7 @@ def application_json(self): # XXX: This isn't technically correct, but we'll hope for the best. # Patches welcome! - # Insert new media-type handlers here + # Insert new Response media-type handlers here class RequestBody(Body): ''' From 51f5b78a08b650b12d6dffacce3d1b27a5c8d650 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 20:37:24 -0400 Subject: [PATCH 14/18] test: Make it directly executable by Unix systems --- test.py | 1 + 1 file changed, 1 insertion(+) mode change 100644 => 100755 test.py diff --git a/test.py b/test.py old mode 100644 new mode 100755 index 60d7377..e8e796e --- a/test.py +++ b/test.py @@ -1,3 +1,4 @@ +#!/usr/bin/env python3 from __future__ import print_function from agithub import GitHub From e9cd78dc203a0f5b0b772b88937b152a8073f601 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 20:55:12 -0400 Subject: [PATCH 15/18] Set default request content-type to 'application/json' Before the RequestBody was introduced, all request bodies were assumed to be JSON data. We'd like to keep this behavior because * JSON is expected to be a common value for this field * It's one less thing existing users will need to upgrade - Add 'content-type' to the DEFAULT_HEADERS request - Delete the content-type header in case there is no body This allows non-body methods (like get, head) to work unchanged. (Sending a content-type header when no body is expected might theoretically break some servers.) --- agithub.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/agithub.py b/agithub.py index a2a5a54..75764b6 100644 --- a/agithub.py +++ b/agithub.py @@ -23,6 +23,7 @@ # objects.) DEFAULT_HEADERS = { 'user-agent': 'agithub/' + STR_VERSION + , 'content-type' : 'application/json' } class API(object): @@ -223,6 +224,11 @@ def request(self, method, url, body, headers): headers = self.updateWithDefaultHeaders(headers) + if body is None: + # Sending a content-type header wo/body might break some + # servers. Is this far-fetched? + del headers['content-type'] + if self.username: headers['authorization'] = self.authHeader From 6a44c59509ecb9486b14f73551c66a4dfc2f07d6 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sat, 14 Jun 2014 20:40:12 -0400 Subject: [PATCH 16/18] Update ChangeLog - TODO: Reuse TCP connections - Update "Parse Content-Type" bullet --- ChangeLog.md | 20 ++++++++++++++++++-- agithub.py | 1 + 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 9438ca2..b4a4418 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -23,6 +23,21 @@ Upcoming Unscheduled ----------- +* Support reusing TCP connections, and "pipelining" of requests, a la + RFC 2068, Sect 8.1, L2377 + + - The user must ask for pipelining, and supply a callback function + to be called after a response is received. + - Rename Client.request() -> Client.addRequest() (or such) + - Have Client.addRequest() check whether a persistent connection is + wanted, and save the entire request in a Client.pendingRequests + list in that case + - Create a new Client.sendRequests() method which the user may call + to send all requests up the pipeline. (It should work even if the + server does not support pipelining) + - Call the user-supplied callback whenever a request is received. + There are some concurrency issues here, and we may elect to call + the callback only after *all* requests are received. * Create a script to pack the basic module and any given set of service-specific classes as one file @@ -89,8 +104,9 @@ v2.0 - Does GitHub support this? It should. And if so, we should use it by default -* Parse Content-Type header more correctly; make a dict of the - parameters available to the media-type handlers +* Parse Content-Type header correctly; make a dict of the + parameters (Content.ctypeParameters) available to the media-type + handlers v1.1.1 ------ diff --git a/agithub.py b/agithub.py index 75764b6..65f73db 100644 --- a/agithub.py +++ b/agithub.py @@ -232,6 +232,7 @@ def request(self, method, url, body, headers): if self.username: headers['authorization'] = self.authHeader + reqBody = RequestBody(body, headers) conn = self.getConnection() conn.request(method, url, reqBody.process(), headers) From 3d373435c8110612cad061e9a9b31a7a1abd752c Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sun, 15 Jun 2014 11:58:33 -0400 Subject: [PATCH 17/18] ResponseBody: New media-types application/xml, text/xml We use xml.dom.minidom to de-serialize the results. --- agithub.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/agithub.py b/agithub.py index 65f73db..0ebff8a 100644 --- a/agithub.py +++ b/agithub.py @@ -5,6 +5,8 @@ import re from functools import partial, update_wrapper +import xml.dom.minidom + import sys if sys.version_info[0:2] > (3,0): import http.client @@ -388,6 +390,22 @@ def application_json(self): # XXX: This isn't technically correct, but we'll hope for the best. # Patches welcome! + def application_xml(self): + self.decodeBody() + + try: + pybody = xml.dom.minidom.parseString(self.body) + except Exception: #TODO: What kind of exceptions? + pybody = self.body + + return pybody + + + text_xml = application_xml + # The difference between text/xml and application/xml is whether it + # is human-readable or not. For our purposes, there is no + # difference. RFC 3023, L270. + # Insert new Response media-type handlers here class RequestBody(Body): From e1143ed97638f18ccb17f7c409641100ffa58f92 Mon Sep 17 00:00:00 2001 From: Jonathan Paugh Date: Sun, 15 Jun 2014 12:02:02 -0400 Subject: [PATCH 18/18] Fix Client.setConnectionProperties: unconditionally set self.defaultHeaders In case the API does not supply extra_headers, self.request() will fail trying to delete the 'content-type' header from them. Including the DEFAULT_HEADERS (always) ensures that 'content-type' is defined --- agithub.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agithub.py b/agithub.py index 0ebff8a..c344c7f 100644 --- a/agithub.py +++ b/agithub.py @@ -185,8 +185,8 @@ def setConnectionProperties(self, props): raise TypeError("Client.setConnectionProperties: Expected ConnectionProperties object") self.prop = props + self.defaultHeaders = DEFAULT_HEADERS.copy() if self.prop.extra_headers is not None: - self.defaultHeaders = DEFAULT_HEADERS.copy() self.defaultHeaders.update(self.prop.extra_headers) # Enforce case restrictions on self.defaultHeaders