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 _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
