[openstack-dev] [Neutron][LBaaS] Improvements to current reviews

Brandon Logan brandon.logan at RACKSPACE.COM
Sat Aug 9 21:29:27 UTC 2014


So I've done some work on improving the code on the current pending
reviews.  And would like to get peoples' opinions on whether I should
add antoher patch set to those reviews, or add the changes as as another
review dependent on the pending ones.

To be clear, no matter what the first review in the chain will not
change:
https://review.openstack.org/#/c/105331/

However, if adding another patch set is preferrable then plugin and db
implementation review would get another patch set and then obviously
anything depending on that.

https://review.openstack.org/#/c/105609/

My opinion is that I'd like to get both of these in as a new patch set.
Reason being that the reviews don't have any +2's and there is
uncertainty because of the GBP discussion.  So, I don't think it'd be a
major issue if a new patch set was created.  Speak up if you think
otherwise.  I'd like to get as many people's thoughts as possible.

The changes are:

1) Added data models, which are just plain python object mimicking the
sql alchemy models, but don't have the overhead or dynamic nature of
being sql alchemy models.  These data models are now returned by the
database methods, instead of the sql alchemy objects.  Also, I moved the
definition of the sql alchemy models into its own module.  I've been
wanting to do this but since I thought I was running out of time I left
it for later.

These shouldn't cause many merge/rebase conflicts, but it probably cause
a few because the sql alchemy models were moved to a different module.


2) The LoadBalancerPluginv2 no longer inherits from the
LoadBalancerPluginDbv2.  The database is now a composite attribute of
the plugin (i.e. plugin.db.get_loadbalancer()).  This cleans up the code
a bit and removes the necessity for _delete_db_entity methods that the
drivers needed because now they can actually call the database methods.
Also, this makes testing more clear, though I have not added any tests
for this because the previous tests are sufficient for now.  Adding the
appropriate tests would add 1k or 2k lines most likely.

This will likely cause more conflicts because the _db_delete_entity
methods have been removed.  However, the new driver interface/mixins
defined a db_method for all drivers to use, so if that is being used
there shouldn't be any problems.

Thanks,
Brandon






More information about the OpenStack-dev mailing list