[openstack-dev] [Neutron][LBaaS] Improvements to current reviews
gkotton at vmware.com
Sun Aug 10 12:18:35 UTC 2014
I took a look at https://review.openstack.org/#/c/105331/ and had one minor issue which I think can be addressed. Prior to approving we need to understand what the state of the V2 API will be.
From: Vijay Venkatachalam <Vijay.Venkatachalam at citrix.com<mailto:Vijay.Venkatachalam at citrix.com>>
Reply-To: OpenStack List <openstack-dev at lists.openstack.org<mailto:openstack-dev at lists.openstack.org>>
Date: Sunday, August 10, 2014 at 2:57 PM
To: OpenStack List <openstack-dev at lists.openstack.org<mailto:openstack-dev at lists.openstack.org>>
Subject: Re: [openstack-dev] [Neutron][LBaaS] Improvements to current reviews
Thanks Brandon for constant improvisation.
I agree with Doug. Please update current one. We already hv more number of reviews :-). It will be difficult to manage if we add more.
Sent using CloudMagic
On Sun, Aug 10, 2014 at 3:23 AM, Doug Wiegley <dougw at a10networks.com<mailto:dougw at a10networks.com>> wrote:
I think you should update the current reviews (new patch set, not additional review.)
> On Aug 9, 2014, at 3:34 PM, "Brandon Logan" <brandon.logan at RACKSPACE.COM<mailto:brandon.logan at RACKSPACE.COM>> wrote:
> 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
> 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.
> 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.
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org<mailto:OpenStack-dev at lists.openstack.org>
OpenStack-dev mailing list
OpenStack-dev at lists.openstack.org<mailto:OpenStack-dev at lists.openstack.org>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the OpenStack-dev