Code review comment for lp:~rackspace-titan/nova/osapi-serialization

> >> Diff lines 1928 & 1946 should not be comparing type(x) to a type; instead,
> use
> >> isinstance().
>
> > So these are "legacy" conditionals that have been working here since before
> I was
> > on the project. This branch is targeted at refactoring the organization of
> the
> > wsgi-related objects, not rewriting our generic xml serialization. After
> hearing
> > that, are you okay with me leaving it?
>
> If you add a #TODO to those lines, sure.

I'm not sure what I would say the TODO is. I don't see a clear path to "fix" this, and I think most people could argue both sides of the type vs. isinstance debate.

> >> If that's correct, the whole structure could be written more
> >> simply as:
> >> action_method = getattr(self, action, self.default)
>
> > Good point. I typically use Exception when I don't know what exactly I may
> need to catch.
>
> Trick: what you do in those cases is write it:
> except Exception as e:
> print "EXCEPTION", type(e)
> ...and then run the test for that code, passing in the bad condition that
> would trigger the exception. Once it's triggered, you'll know the type, and
> can change the code to catch the correct exception class.

By not knowing the type, I am referring more to the case where literally anything could be thrown. For example, a nova.api.openstack.wsgi.Controller object could define its create method to throw literally anything.

> I still think that you should change the lines (1787 and 1881) from:
> try:
> action_method = getattr(self, action)
> except (AttributeError, TypeError):
> action_method = self.default
> to:
> action_method = getattr(self, action, self.default)
> After all, that's what the default parameter for getattr() is there for!

Wow, I need to read over the built-ins doc. I had no idea there was a default parameter. Fixed!