On Tue, Oct 18, 2011 at 12:47 PM, Caio Romão <[email protected]> wrote: > Hey all, > > Short introduction: I've been hanging out on IRC for a while, nickname > errado, and I want to dedicate some of my time for libcloud :) > > A couple of weeks ago I've created a patchset which simply does as the > ticket describes: Creates the {Json,Xml}Response clases and set the > drivers to use them (it's the "response-hierarchy" branch from > https://github.com/caio/libcloud -- It doesn't cleanly merge on trunk > currently - I'll gladly fix this if there's interest in merging it > before what I discuss below). > > While talking to mirrorbox (never got your real name, sorry) on > #libcloud he suggested adding a request validation mechanism to move > away all those verification steps from parse_body(), which would > definitely help in the code organization. > > There are some things that I think we should take into account when/if > refactoring the Request behavior: > > 1. Some drivers allow an empty body (returning `None` on parse_body > when this happens) > 2. Brightbox driver checks the content-type header and if not json, > returns the unparsed self.body > 3. Dreamhost driver raises an exception if the properly parsed body > doesn't contain 'result' => 'success' (Rimuhosting driver does > something similar) > 4. Some drivers return the unparsed body in case of an error, others > throw an exception > > Anyway, I believe some of those are done to circumvent bugs/API > oddities, but I think libcloud would benefit from standardizing (where > possible) how Request events are handled and this is where the "RFC" > in the title comes from. Would that be interesting? Anyone has any > ideas on this front? Can I work on that/ >
In general yes, this is a very good thing to do. The examples of returning the body unparsed if its an unknown content type was done in several drivers because for example, they might return a 403, with a plain text message, but the rest of the API returns JSON. Some of those type cases don't have good test case coverage, and the response classes were kinda hacked around with as you can see in the inconsistenties. I think a base class for xml and json definitely makes sense, it just needs to be careful and have a few methods for falling back reasonably to not-parsing things
