* Not much we can do here, but looks like this is gonna conflict pretty heavily with `related_images`
* I'm seeing quite a bit of dependency-injection being used. This can usually be avoid by stubbing out using Fakes and Mocks. This keeps the implementation clean since it won't have to take `client`, `image_service`, `compute_service` args. This isn't a deal-breaker for this patch (by any means), but wanted to raise it for discussion.
Specifics
=========
> 235 + ex = webob.exc.HTTPNotFound(explanation="Image not found.")
It's usually not a good idea to use now-now in tests, since the tests, in a way, change each time. For example, you can see a test pass everyday but fail on Feb. 29th. Rather, you might want to do something like:
Nice work Brian. A few comments:
General
=======
* Not much we can do here, but looks like this is gonna conflict pretty heavily with `related_images`
* I'm seeing quite a bit of dependency- injection being used. This can usually be avoid by stubbing out using Fakes and Mocks. This keeps the implementation clean since it won't have to take `client`, `image_service`, `compute_service` args. This isn't a deal-breaker for this patch (by any means), but wanted to raise it for discussion.
Specifics
=========
> 235 + ex = webob.exc. HTTPNotFound( explanation= "Image not found.")
Needs i18n treatment.
> 238 + return dict(image= self.get_ builder( req).build( image, True))
Might be a little clearer if a kwarg is used, like:
return dict(image= self.get_ builder( req).build( image, detail=True))
> 464 now = datetime. datetime. utcnow( )
It's usually not a good idea to use now-now in tests, since the tests, in a way, change each time. For example, you can see a test pass everyday but fail on Feb. 29th. Rather, you might want to do something like:
NOW_DATETIME = datetime. datetime( 2010, 10, 11, 10, 32, 23) 11T10:32: 23Z"
NOW_STR = "2010-10-