Open Stack

" I think you should update the current reviews (new patch set, not additional review.)"
+1

I like those changes: +2

-----Original Message-----
From: Doug Wiegley [mailto:dougw at a10networks.com] 
Sent: Sunday, August 10, 2014 12:51 AM
To: OpenStack Development Mailing List (not for usage questions)
Subject: Re: [openstack-dev] [Neutron][LBaaS] Improvements to current reviews

I think you should update the current reviews (new patch set, not additional review.)

Doug


> On Aug 9, 2014, at 3:34 PM, "Brandon Logan" <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
> 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 at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

_______________________________________________
OpenStack-dev mailing list
OpenStack-dev at lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev