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 :-)

ya.

> > 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?

nope, will fix. :)

> + with tempfile.NamedTemporaryFile() as f:

Copied from Swift ;) I'll rename to temp_file.

> 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?)