<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<p dir="ltr">Thanks Brandon for constant improvisation.</p>
<p dir="ltr">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.
<br>
</p>
<p dir="ltr">Thanks,<br>
Vijay</p>
<p dir="ltr">Sent using CloudMagic</p>
<br>
<div class="cm_quote" style=" color: #787878">On Sun, Aug 10, 2014 at 3:23 AM, Doug Wiegley <<a href="mailto:dougw@a10networks.com">dougw@a10networks.com</a>> wrote:</div>
<br>
<div id="oldcontent" style="background-image: initial; background-attachment: initial; background-origin: initial; background-clip: initial; background-color: rgb(255, 255, 255); background-position: initial initial; background-repeat: initial initial; ">
<blockquote style="">
<p dir="ltr">I think you should update the current reviews (new patch set, not additional review.)
<br>
<br>
Doug <br>
<br>
<br>
> On Aug 9, 2014, at 3:34 PM, "Brandon Logan" <brandon.logan@RACKSPACE.COM> wrote:
<br>
> <br>
> So I've done some work on improving the code on the current pending <br>
> reviews. And would like to get peoples' opinions on whether I should <br>
> add antoher patch set to those reviews, or add the changes as as another <br>
> review dependent on the pending ones. <br>
> <br>
> To be clear, no matter what the first review in the chain will not <br>
> change: <br>
> https://review.openstack.org/#/c/105331/ <br>
> <br>
> However, if adding another patch set is preferrable then plugin and db <br>
> implementation review would get another patch set and then obviously <br>
> anything depending on that. <br>
> <br>
> https://review.openstack.org/#/c/105609/ <br>
> <br>
> My opinion is that I'd like to get both of these in as a new patch set. <br>
> Reason being that the reviews don't have any +2's and there is <br>
> uncertainty because of the GBP discussion. So, I don't think it'd be a <br>
> major issue if a new patch set was created. Speak up if you think <br>
> otherwise. I'd like to get as many people's thoughts as possible. <br>
> <br>
> The changes are: <br>
> <br>
> 1) Added data models, which are just plain python object mimicking the <br>
> sql alchemy models, but don't have the overhead or dynamic nature of <br>
> being sql alchemy models. These data models are now returned by the <br>
> database methods, instead of the sql alchemy objects. Also, I moved the <br>
> definition of the sql alchemy models into its own module. I've been <br>
> wanting to do this but since I thought I was running out of time I left <br>
> it for later. <br>
> <br>
> These shouldn't cause many merge/rebase conflicts, but it probably cause <br>
> a few because the sql alchemy models were moved to a different module. <br>
> <br>
> <br>
> 2) The LoadBalancerPluginv2 no longer inherits from the <br>
> LoadBalancerPluginDbv2. The database is now a composite attribute of <br>
> the plugin (i.e. plugin.db.get_loadbalancer()). This cleans up the code <br>
> a bit and removes the necessity for _delete_db_entity methods that the <br>
> drivers needed because now they can actually call the database methods. <br>
> Also, this makes testing more clear, though I have not added any tests <br>
> for this because the previous tests are sufficient for now. Adding the <br>
> appropriate tests would add 1k or 2k lines most likely. <br>
> <br>
> This will likely cause more conflicts because the _db_delete_entity <br>
> methods have been removed. However, the new driver interface/mixins <br>
> defined a db_method for all drivers to use, so if that is being used <br>
> there shouldn't be any problems. <br>
> <br>
> Thanks, <br>
> Brandon <br>
> <br>
> <br>
> <br>
> <br>
> _______________________________________________ <br>
> OpenStack-dev mailing list <br>
> OpenStack-dev@lists.openstack.org <br>
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev <br>
<br>
_______________________________________________ <br>
OpenStack-dev mailing list <br>
OpenStack-dev@lists.openstack.org <br>
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev <br>
</p>
</blockquote>
</div>
</body>
</html>