You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Syed Ahmed <sa...@cloudops.com> on 2013/11/13 19:07:45 UTC
Review Request 15489: Adding protocol parameter to loadbalancer response
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15489/
-----------------------------------------------------------
Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
Repository: cloudstack-git
Description
-------
Adding protocol parameter to Loadbalancer Response
Diffs
-----
api/src/com/cloud/network/rules/LoadBalancer.java e6dadca
api/src/com/cloud/network/rules/LoadBalancerContainer.java 9d5ea59
api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java 0f18097
engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java 37a747e
server/src/com/cloud/api/ApiResponseHelper.java 903c485
Diff: https://reviews.apache.org/r/15489/diff/
Testing
-------
Thanks,
Syed Ahmed
Re: Review Request 15489: Adding protocol parameter to loadbalancer
response
Posted by Syed Ahmed <sa...@cloudops.com>.
Thanks Murali.
On Tue 19 Nov 2013 01:10:45 PM EST, Murali Reddy wrote:
> I have cherry-picked to 4.3. Will be pushing in a while.
>
> On 19/11/13 11:25 PM, "Syed Ahmed" <sa...@cloudops.com> wrote:
>
>> Hi Murali,
>>
>> Should I send a patch for the 4.3 branch as well?
>>
>> Thanks,
>> -Syed
>>
>> On Tue 19 Nov 2013 12:01:53 PM EST, Murali Reddy wrote:
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/15489/
>>>
>>>
>>> Ship it!
>>>
>>> commit 481af07fb130d24490efe6a98970cb13104e4046
>>>
>>> - Murali Reddy
>>>
>>>
>>> On November 13th, 2013, 6:08 p.m. UTC, Syed Ahmed wrote:
>>>
>>> Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
>>> By Syed Ahmed.
>>>
>>> /Updated Nov. 13, 2013, 6:08 p.m./
>>>
>>> *Repository: * cloudstack-git
>>>
>>>
>>> Description
>>>
>>> Adding protocol parameter to Loadbalancer Response
>>>
>>>
>>> Diffs
>>>
>>> * api/src/com/cloud/network/rules/LoadBalancer.java (e6dadca)
>>> * api/src/com/cloud/network/rules/LoadBalancerContainer.java (9d5ea59)
>>> * api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java
>>> (0f18097)
>>> *
>>> engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.
>>> java
>>> (37a747e)
>>> * server/src/com/cloud/api/ApiResponseHelper.java (903c485)
>>>
>>> View Diff <https://reviews.apache.org/r/15489/diff/>
>>>
>>
>>
>>
>
>
Re: Review Request 15489: Adding protocol parameter to loadbalancer
response
Posted by Alena Prokharchyk <Al...@citrix.com>.
Yes, Syed, it should go both to 4.3 and master branches.
-alena.
From: Syed Ahmed <sa...@cloudops.com>>
Date: Tuesday, November 19, 2013 9:55 AM
To: Murali Reddy <mu...@gmail.com>>
Cc: cloudstack <de...@cloudstack.apache.org>>, Alena Prokharchyk <al...@citrix.com>>
Subject: Re: Review Request 15489: Adding protocol parameter to loadbalancer response
Hi Murali,
Should I send a patch for the 4.3 branch as well?
Thanks,
-Syed
On Tue 19 Nov 2013 12:01:53 PM EST, Murali Reddy wrote:
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15489/
Ship it!
commit 481af07fb130d24490efe6a98970cb13104e4046
- Murali Reddy
On November 13th, 2013, 6:08 p.m. UTC, Syed Ahmed wrote:
Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
By Syed Ahmed.
/Updated Nov. 13, 2013, 6:08 p.m./
*Repository: * cloudstack-git
Description
Adding protocol parameter to Loadbalancer Response
Diffs
* api/src/com/cloud/network/rules/LoadBalancer.java (e6dadca)
* api/src/com/cloud/network/rules/LoadBalancerContainer.java (9d5ea59)
* api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java
(0f18097)
* engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java
(37a747e)
* server/src/com/cloud/api/ApiResponseHelper.java (903c485)
View Diff <https://reviews.apache.org/r/15489/diff/>
Re: Review Request 15489: Adding protocol parameter to loadbalancer
response
Posted by Murali Reddy <Mu...@citrix.com>.
I have cherry-picked to 4.3. Will be pushing in a while.
On 19/11/13 11:25 PM, "Syed Ahmed" <sa...@cloudops.com> wrote:
>Hi Murali,
>
>Should I send a patch for the 4.3 branch as well?
>
>Thanks,
>-Syed
>
>On Tue 19 Nov 2013 12:01:53 PM EST, Murali Reddy wrote:
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/15489/
>>
>>
>> Ship it!
>>
>> commit 481af07fb130d24490efe6a98970cb13104e4046
>>
>> - Murali Reddy
>>
>>
>> On November 13th, 2013, 6:08 p.m. UTC, Syed Ahmed wrote:
>>
>> Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
>> By Syed Ahmed.
>>
>> /Updated Nov. 13, 2013, 6:08 p.m./
>>
>> *Repository: * cloudstack-git
>>
>>
>> Description
>>
>> Adding protocol parameter to Loadbalancer Response
>>
>>
>> Diffs
>>
>> * api/src/com/cloud/network/rules/LoadBalancer.java (e6dadca)
>> * api/src/com/cloud/network/rules/LoadBalancerContainer.java (9d5ea59)
>> * api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java
>> (0f18097)
>> *
>>engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.
>>java
>> (37a747e)
>> * server/src/com/cloud/api/ApiResponseHelper.java (903c485)
>>
>> View Diff <https://reviews.apache.org/r/15489/diff/>
>>
>
>
>
Re: Review Request 15489: Adding protocol parameter to loadbalancer
response
Posted by Syed Ahmed <sa...@cloudops.com>.
Hi Murali,
Should I send a patch for the 4.3 branch as well?
Thanks,
-Syed
On Tue 19 Nov 2013 12:01:53 PM EST, Murali Reddy wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15489/
>
>
> Ship it!
>
> commit 481af07fb130d24490efe6a98970cb13104e4046
>
> - Murali Reddy
>
>
> On November 13th, 2013, 6:08 p.m. UTC, Syed Ahmed wrote:
>
> Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
> By Syed Ahmed.
>
> /Updated Nov. 13, 2013, 6:08 p.m./
>
> *Repository: * cloudstack-git
>
>
> Description
>
> Adding protocol parameter to Loadbalancer Response
>
>
> Diffs
>
> * api/src/com/cloud/network/rules/LoadBalancer.java (e6dadca)
> * api/src/com/cloud/network/rules/LoadBalancerContainer.java (9d5ea59)
> * api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java
> (0f18097)
> * engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java
> (37a747e)
> * server/src/com/cloud/api/ApiResponseHelper.java (903c485)
>
> View Diff <https://reviews.apache.org/r/15489/diff/>
>
Re: Review Request 15489: Adding protocol parameter to loadbalancer response
Posted by Murali Reddy <mu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15489/#review29115
-----------------------------------------------------------
Ship it!
commit 481af07fb130d24490efe6a98970cb13104e4046
- Murali Reddy
On Nov. 13, 2013, 6:08 p.m., Syed Ahmed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15489/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2013, 6:08 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding protocol parameter to Loadbalancer Response
>
>
> Diffs
> -----
>
> api/src/com/cloud/network/rules/LoadBalancer.java e6dadca
> api/src/com/cloud/network/rules/LoadBalancerContainer.java 9d5ea59
> api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java 0f18097
> engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java 37a747e
> server/src/com/cloud/api/ApiResponseHelper.java 903c485
>
> Diff: https://reviews.apache.org/r/15489/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Syed Ahmed
>
>
Re: Review Request 15489: Adding protocol parameter to loadbalancer response
Posted by Alena Prokharchyk <al...@citrix.com>.
> On Nov. 15, 2013, 1:53 p.m., Murali Reddy wrote:
> > Syed I dont think you added 'protocol' to createLoadBalancerRule. There should not be 'protocol' in the LoadBalancerContainer?
>
> Syed Ahmed wrote:
> I actually did add the 'protocol' parameter to createLoadBalancerRule. This maps to `lb_protocol` field in the `load_balancing_rules` table.
>
> I added getLbProtocol() to LoabalacnerContainer because I thought that is the right palce as other parameters like algorithm ( getAlgorithm() ) scheme etc are present here.
>
> Does this sound correct?
Syed, that sounds correct to me. Container should have the properties algorithm, protocol, public Ip, etc. While LoadBalancer represents only the rule itself (the port combination)
- Alena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15489/#review28968
-----------------------------------------------------------
On Nov. 13, 2013, 6:08 p.m., Syed Ahmed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15489/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2013, 6:08 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding protocol parameter to Loadbalancer Response
>
>
> Diffs
> -----
>
> api/src/com/cloud/network/rules/LoadBalancer.java e6dadca
> api/src/com/cloud/network/rules/LoadBalancerContainer.java 9d5ea59
> api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java 0f18097
> engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java 37a747e
> server/src/com/cloud/api/ApiResponseHelper.java 903c485
>
> Diff: https://reviews.apache.org/r/15489/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Syed Ahmed
>
>
Re: Review Request 15489: Adding protocol parameter to loadbalancer
response
Posted by Alena Prokharchyk <Al...@citrix.com>.
Syed, that sounds correct to me. Container should have the properties algorithm, protocol, public Ip, etc. While LoadBalancer represents only the rule itself (the port combination)
-Alena.
From: Syed Ahmed <sa...@cloudops.com>>
Reply-To: Syed Ahmed <sa...@cloudops.com>>
Date: Friday, November 15, 2013 9:21 AM
To: Murali Reddy <mu...@gmail.com>>, Alena Prokharchyk <al...@citrix.com>>
Cc: Syed Ahmed <sa...@cloudops.com>>, cloudstack <de...@cloudstack.apache.org>>
Subject: Re: Review Request 15489: Adding protocol parameter to loadbalancer response
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15489/
On November 15th, 2013, 1:53 p.m. UTC, Murali Reddy wrote:
Syed I dont think you added 'protocol' to createLoadBalancerRule. There should not be 'protocol' in the LoadBalancerContainer?
I actually did add the 'protocol' parameter to createLoadBalancerRule. This maps to `lb_protocol` field in the `load_balancing_rules` table.
I added getLbProtocol() to LoabalacnerContainer because I thought that is the right palce as other parameters like algorithm ( getAlgorithm() ) scheme etc are present here.
Does this sound correct?
- Syed
On November 13th, 2013, 6:08 p.m. UTC, Syed Ahmed wrote:
Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
By Syed Ahmed.
Updated Nov. 13, 2013, 6:08 p.m.
Repository: cloudstack-git
Description
Adding protocol parameter to Loadbalancer Response
Diffs
* api/src/com/cloud/network/rules/LoadBalancer.java (e6dadca)
* api/src/com/cloud/network/rules/LoadBalancerContainer.java (9d5ea59)
* api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java (0f18097)
* engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java (37a747e)
* server/src/com/cloud/api/ApiResponseHelper.java (903c485)
View Diff<https://reviews.apache.org/r/15489/diff/>
Re: Review Request 15489: Adding protocol parameter to loadbalancer response
Posted by Syed Ahmed <sa...@cloudops.com>.
> On Nov. 15, 2013, 1:53 p.m., Murali Reddy wrote:
> > Syed I dont think you added 'protocol' to createLoadBalancerRule. There should not be 'protocol' in the LoadBalancerContainer?
I actually did add the 'protocol' parameter to createLoadBalancerRule. This maps to `lb_protocol` field in the `load_balancing_rules` table.
I added getLbProtocol() to LoabalacnerContainer because I thought that is the right palce as other parameters like algorithm ( getAlgorithm() ) scheme etc are present here.
Does this sound correct?
- Syed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15489/#review28968
-----------------------------------------------------------
On Nov. 13, 2013, 6:08 p.m., Syed Ahmed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15489/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2013, 6:08 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding protocol parameter to Loadbalancer Response
>
>
> Diffs
> -----
>
> api/src/com/cloud/network/rules/LoadBalancer.java e6dadca
> api/src/com/cloud/network/rules/LoadBalancerContainer.java 9d5ea59
> api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java 0f18097
> engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java 37a747e
> server/src/com/cloud/api/ApiResponseHelper.java 903c485
>
> Diff: https://reviews.apache.org/r/15489/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Syed Ahmed
>
>
Re: Review Request 15489: Adding protocol parameter to loadbalancer response
Posted by Murali Reddy <mu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15489/#review28968
-----------------------------------------------------------
Syed I dont think you added 'protocol' to createLoadBalancerRule. There should not be 'protocol' in the LoadBalancerContainer?
- Murali Reddy
On Nov. 13, 2013, 6:08 p.m., Syed Ahmed wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15489/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2013, 6:08 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding protocol parameter to Loadbalancer Response
>
>
> Diffs
> -----
>
> api/src/com/cloud/network/rules/LoadBalancer.java e6dadca
> api/src/com/cloud/network/rules/LoadBalancerContainer.java 9d5ea59
> api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java 0f18097
> engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java 37a747e
> server/src/com/cloud/api/ApiResponseHelper.java 903c485
>
> Diff: https://reviews.apache.org/r/15489/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Syed Ahmed
>
>
Re: Review Request 15489: Adding protocol parameter to loadbalancer response
Posted by Syed Ahmed <sa...@cloudops.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15489/
-----------------------------------------------------------
(Updated Nov. 13, 2013, 6:08 p.m.)
Review request for cloudstack, Alena Prokharchyk and Murali Reddy.
Changes
-------
Adding branch
Repository: cloudstack-git
Description
-------
Adding protocol parameter to Loadbalancer Response
Diffs
-----
api/src/com/cloud/network/rules/LoadBalancer.java e6dadca
api/src/com/cloud/network/rules/LoadBalancerContainer.java 9d5ea59
api/src/org/apache/cloudstack/api/response/LoadBalancerResponse.java 0f18097
engine/schema/src/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java 37a747e
server/src/com/cloud/api/ApiResponseHelper.java 903c485
Diff: https://reviews.apache.org/r/15489/diff/
Testing
-------
Thanks,
Syed Ahmed