Code review comment for lp:~jaypipes/glance/bug704854

Jay-

Sorry I'm late on reviewing this, overall this looks really good. Surprised by the decorator only validating on update (major WTF).

Femto-nits:

> 170 + type = values.get('type', None)

We probably shouldn't be using `type` as a variable name (since it's a builtin). `type_` seems ok, `image_type` seems better. (Was burned by this recently :-)

> 320 + logger.debug("Uploading image data for image %(image_id)s "
> 321 + "to %(store_name)s store"
> 322 + % {'image_id': image_id,
> 323 + 'store_name': image_store})

Certainly not a problem, but is there a reason you didn't use the ` % locals()` idiom here?

+ with tempfile.NamedTemporaryFile() as f:

Rather than just `f`, I think `temp_file` or `tmp` would be a little clearer and help with grep-ability.

(Should we eliminate single-letter variable names entirely since they are usually a bad idea? Should we special case the counting variables `i` and `j` since they are *soo* common/idiomatic?)