<div dir="ltr">I agree with Doug as well. We should update the current patch. Susanne</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Aug 10, 2014 at 8:18 AM, Gary Kotton <span dir="ltr"><<a href="mailto:gkotton@vmware.com" target="_blank">gkotton@vmware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word;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" target="_blank">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>
<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" target="_blank">Vijay.Venkatachalam@citrix.com</a>><br>
<span style="font-weight:bold">Reply-To: </span>OpenStack List <<a href="mailto:openstack-dev@lists.openstack.org" target="_blank">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" target="_blank">openstack-dev@lists.openstack.org</a>><div class=""><br>
<span style="font-weight:bold">Subject: </span>Re: [openstack-dev] [Neutron][LBaaS] Improvements to current reviews<br>
</div></div><div><div class="h5">
<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 style="color:#787878">On Sun, Aug 10, 2014 at 3:23 AM, Doug Wiegley <<a href="mailto:dougw@a10networks.com" target="_blank">dougw@a10networks.com</a>> wrote:</div>
<br>
<div style="background-image:initial;background-color:rgb(255,255,255);background-repeat:initial initial">
<blockquote>
<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" target="_blank">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/" target="_blank">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/" target="_blank">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" target="_blank">OpenStack-dev@lists.openstack.org</a>
<br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">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" target="_blank">OpenStack-dev@lists.openstack.org</a>
<br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a>
<br>
</p>
</blockquote>
</div>
</div>
</div>
</div></div></span>
</div>
<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" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div>