<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; color: rgb(0, 0, 0); font-size: 14px; font-family: Calibri, sans-serif;">
<div>Hi,</div>
<div>I took a look at <a href="https://review.openstack.org/#/c/105331">https://review.openstack.org/#/c/105331</a>/ 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.</div>
<div>Thanks</div>
<div>Gary</div>
<div><br>
</div>
<span id="OLK_SRC_BODY_SECTION">
<div style="font-family:Calibri; font-size:11pt; text-align:left; color:black; BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">
<span style="font-weight:bold">From: </span>Vijay Venkatachalam <<a href="mailto:Vijay.Venkatachalam@citrix.com">Vijay.Venkatachalam@citrix.com</a>><br>
<span style="font-weight:bold">Reply-To: </span>OpenStack List <<a href="mailto:openstack-dev@lists.openstack.org">openstack-dev@lists.openstack.org</a>><br>
<span style="font-weight:bold">Date: </span>Sunday, August 10, 2014 at 2:57 PM<br>
<span style="font-weight:bold">To: </span>OpenStack List <<a href="mailto:openstack-dev@lists.openstack.org">openstack-dev@lists.openstack.org</a>><br>
<span style="font-weight:bold">Subject: </span>Re: [openstack-dev] [Neutron][LBaaS] Improvements to current reviews<br>
</div>
<div><br>
</div>
<div>
<div>
<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" <<a href="mailto:brandon.logan@RACKSPACE.COM">brandon.logan@RACKSPACE.COM</a>> 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>
> <a href="https://review.openstack.org/#/c/105331/">https://review.openstack.org/#/c/105331/</a>
<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>
> <a href="https://review.openstack.org/#/c/105609/">https://review.openstack.org/#/c/105609/</a>
<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>
> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a>
<br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a>
<br>
<br>
_______________________________________________ <br>
OpenStack-dev mailing list <br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a>
<br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a>
<br>
</p>
</blockquote>
</div>
</div>
</div>
</span>
</body>
</html>