deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph VLcek <jvl...@redhat.com>
Subject Re: Complete rewrite of deltacloud-client
Date Thu, 21 Mar 2013 17:42:09 GMT

On Mar 7, 2013, at 8:10 AM, mfojtik@redhat.com wrote:

> Good news everyone!
> 
> I started a complete rewrite of the deltacloud-client component
> of Deltacloud core repo few week ago, because:
> 
> 1. Too much *magic* in the current client (meta-programming, etc)
> 2. No documentation (see 1))
> 3. Hard to navigate in code and fix bugs (see 1,2)
> 4. Hard to add collection that does not follow the common 'schema'
> 5. Error reporting sux (even we have superb error API in DC-core)
> 
> And since I keep hearing complaints mainly about 5) and 3), I decided
> to change it.
> 
> This brand new client is build using the Faraday[1] library, which do a
> better job in handling errors (using middleware[2]) and there is no magic
> needed to get it handle basic HTTP auth and HTTP headers properly.
> 
> Faraday has also built-in support for VCR (or better VCR has built-in
> 'hook into faraday'), so working with VCR is a pleasure ;-)
> 
> The code structure of new client is following:
> 
> lib
>  - deltacloud
>    - client
>      - models
>      - methods
> 
> The *methods* contains methods ;-) used to communication with Deltacloud API.
> So for example 'create_instance', 'instances', 'images', etc.
> While the *models* contains the XML deserialization part and some models also
> include *method* file, so you can write something like:
> 
> inst = client.instance('foo')
> inst.reboot! # => calls client.reboot_instance(@self)
> 
> Part of this exercise was to keep the codeclimate[3] ratings high, so in other
> words avoid duplication and complexity :-)
> 
> !!! IMPORTANT NOTE !!!
> 
> This patch *might* break existing apps that use the old client. I tried really
> hard[4] to keep all methods from previous client, but there might be some
> explosions. Please let me know if you find some :-)
> 
> For example:
> 
> Deltacloud::Client(url, user, pass) instead of DeltaCloud.new(url, user, pass)
> 
> Some patches might not be sent properly (especially 4/5 with VCR fixtures).
> For that please refer to tracker: http://tracker.deltacloud.org/set/374
> 
> Cheers,
>  Michal
> 
> [1] https://github.com/lostisland/faraday
> [2] https://github.com/lostisland/faraday#writing-middleware
> [3] https://codeclimate.com/github/mifo/deltacloud-client
> [4] https://github.com/mifo/deltacloud-client/blob/master/lib/deltacloud/client/methods/backward_compatiblity.rb
> 


Michal,

This looks great. Many of my comments are really questions regarding
why you used Ruby the way you did. 

I've formated my feedback/questions in the attached patch.
I had initially started making code changes but then decided to 
just leave comments/questions in the code, prefaced wtih "Joe".

I've also done some tpyo and grammar fixes and comment changes to
attempt to make the comments more consistent.

A general question: Are we trying to stick to 80char line lenght?

Hope this helps!
  Joe V.

Mime
  • Unnamed multipart/mixed (inline, None, 0 bytes)
View raw message