Merge lp://qastaging/~danilo/simplestreams/insert-item-factoring into lp://qastaging/simplestreams

Danilo, thank you for cleanups. Looks mostly good.

The one thing i'm concerned about is changing the signature on GlanceMirror.

I'm not opposed to variable name improvements. but maybe something a bit shorrter.
  flattened_img_data
  target_sstream_item

clearly we're in bike shed territory here.

do you think its better or worse to have hypervisor_mapping at the class level ?
rather than getting it in insert_item and passing it around.

Thanks for the review, Scott.

What worries you about the changed signature of GlanceMirror? It's an optional parameter put at the very end, thus no existing call-site could be affected. Dependency injection of "OpenStack module" like this allows for simpler testing without having to re-factor the constructor itself (which does too much anyway). Alternative is to move any calls to openstack.* and whatever is using the returned data outside the constructor, and call that in every method which relies on actual connections (thus following the ideology of only setting up the data in the constructor, but not really doing anything), but that's a bit outside the scope of this branch.

As for hypervisor_mapping, it could very well be at the class level since we initialize GlanceMirror with the config object. However, for helper methods themselves, I prefer to have them rely as little as possible on their "context": makes it clear what their inputs and outputs are (and they are easier to test as well). Thus, I've changed nothing in this regard, but if you prefer it any other way, just shout. :)

I've done the variable renames and pushed that up.

At first i didn't understand or missed the 'openstack=openstack' basically masking the outer scoped import. It seems odd dont you think ? I'm fine with it if you think this sort of thing is "pythonic", but would just as well accept:

    def __init__(self, config, objectstore=None, region=None,
                 name_prefix=None, progress_callback=None,
                 client=None):
     ...
     if client is None:
         client = openstack

I'm willing to defer to you on whatever you think is reasonable. I just find the inner varibable name overriding the module level import at the top easily confusing.

Sure thing: if one person finds it confusing, so might the next one. Used your suggestion instead!