Code review comment for lp:~blamar/nova/openstack-api-1-1-images

> Wow, I've never been told that DI is bad, can you potentially give me more details?

Sure. DI is a pretty common pattern static languages since it gives you the flexibility to test against something other than the original implementation.

With a dynamic language like Python, you get this for free, no dependency injection needed. All you need to do is re-bind the dependent-class in the setUp of the test. I believe we have plenty of examples of this pattern across our test suite. Here's an example:

DI-Style
========

class Dependent(object):
  pass

class DoesSomething(object):
  def __init__(self, dependent=None)
    self.dependent = dependent or Dependent()

class FakeDependent(object):
  pass

# test it
test = DoesSomething(dependent=FakeDependent)

Non-DI-Style
============

class DoesSomething(object):
  def __init__(self)
    self.dependent = Dependent()

# test it
Dependent = FakeDependent() # Rebind, use the `stubs` library in the real case
test = DoesSomething()

Notice that both ways achieve the same thing, the code ultimately uses FakeDependent. But with the Non-DI style, we don't have to add N number of dependents to the constructor.

> I'm not quite sure I follow the logic here as none of the tests do time-based comparison

In this case, that's true. This is more of a suggestion as a best-practice: tests should be invariant over time. I'm just pointing out that there is a hidden dependency on time here which might be come a problem in the future as new tests are added.

FWIW, I've been burnt by this in the past, that's why I'm raising the concern here.

> think that you were potentially not working off a blueprint?

Yeah, my fault here :/ There *is* a blueprint but it's under Glance since that's where most of the work was planning to take place. However, it turned out, it was much cleaner if the changes were made to Nova; at that point I should created a Nova blueprint and rejected the Glance prop. My apologies.