You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Dmitry Batkovich <ba...@gmail.com> on 2014/05/26 16:32:33 UTC
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/
-----------------------------------------------------------
(Updated May 26, 2014, 2:32 p.m.)
Review request for cloudstack, Chiradeep Vittal and daan Hoogland.
Changes
-------
adding relevant reviewers
Repository: cloudstack-git
Description
-------
* CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
* SSL enabling fixed in EC2Engine.java
continuation of https://reviews.apache.org/r/17586/
Diffs
-----
awsapi/conf/ec2-service.properties.in 82f5ad8
awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
awsapi/src/com/cloud/bridge/util/JsonElementUtil.java 2d8afb5
awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
awsapi/src/com/cloud/stack/CloudStackClientException.java ac1b3ef
awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java 8b4f267
Diff: https://reviews.apache.org/r/21776/diff/
Testing
-------
Thanks,
Dmitry Batkovich
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On May 27, 2014, 4:58 p.m., Chiradeep Vittal wrote:
> > Due to all the whitespace and if-block formatting changes, I couldn't find the actually relevant changes.
> > Can you eliminate whitespace and other reformatting?
OK, I have created new review and attach you as reviewer
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review43984
-----------------------------------------------------------
On May 26, 2014, 2:32 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 26, 2014, 2:32 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java 2d8afb5
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackClientException.java ac1b3ef
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java 8b4f267
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Chiradeep Vittal <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review43984
-----------------------------------------------------------
Due to all the whitespace and if-block formatting changes, I couldn't find the actually relevant changes.
Can you eliminate whitespace and other reformatting?
- Chiradeep Vittal
On May 26, 2014, 2:32 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 26, 2014, 2:32 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java 2d8afb5
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackClientException.java ac1b3ef
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java 8b4f267
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On June 24, 2014, 11:10 p.m., Demetrius Tsitrelis wrote:
> > The only change I can see regarding "SSL enabling" is to the getProperty() method for the default value. It looks like that change to null wouldn't matter as the constructor for CloudStackApi() would just reassign 8080?
>
> Dmitry Batkovich wrote:
> Maybe I don't understand you but states of properties "cloudAPISSLEnabled" and "cloudAPIPort" are independent. And port value not depends on "cloudAPISSLEnabled" value. (I will assume that you look at not my code)
>
> Demetrius Tsitrelis wrote:
> Sorry Dmitry - I missed a section of code.
>
> Related to the port number changes you made is that now in the constructor for CloudstackClient there is no way for a user to set the port if SSL is used; you have hard-coded port 443.
No, I didn't ,see https://hc.apache.org/httpclient-3.x/apidocs/org/apache/commons/httpclient/HostConfiguration.html#setHost(java.lang.String, int, org.apache.commons.httpclient.protocol.Protocol)
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46589
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On June 24, 2014, 11:10 p.m., Demetrius Tsitrelis wrote:
> > The only change I can see regarding "SSL enabling" is to the getProperty() method for the default value. It looks like that change to null wouldn't matter as the constructor for CloudStackApi() would just reassign 8080?
Maybe I don't understand you but states of properties "cloudAPISSLEnabled" and "cloudAPIPort" are independent. And port value not depends on "cloudAPISSLEnabled" value. (I will assume that you look at not my code)
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46589
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Demetrius Tsitrelis <dt...@live.com>.
> On June 24, 2014, 11:10 p.m., Demetrius Tsitrelis wrote:
> > The only change I can see regarding "SSL enabling" is to the getProperty() method for the default value. It looks like that change to null wouldn't matter as the constructor for CloudStackApi() would just reassign 8080?
>
> Dmitry Batkovich wrote:
> Maybe I don't understand you but states of properties "cloudAPISSLEnabled" and "cloudAPIPort" are independent. And port value not depends on "cloudAPISSLEnabled" value. (I will assume that you look at not my code)
>
> Demetrius Tsitrelis wrote:
> Sorry Dmitry - I missed a section of code.
>
> Related to the port number changes you made is that now in the constructor for CloudstackClient there is no way for a user to set the port if SSL is used; you have hard-coded port 443.
>
> Dmitry Batkovich wrote:
> No, I didn't ,see https://hc.apache.org/httpclient-3.x/apidocs/org/apache/commons/httpclient/HostConfiguration.html#setHost(java.lang.String, int, org.apache.commons.httpclient.protocol.Protocol)
>
>
> Demetrius Tsitrelis wrote:
> I mean this change:
>
> if (sslEnabled) {
> httpClient.getHostConfiguration().setHost(host, port, new Protocol("https", new EasySSLProtocolSocketFactory(), 443));
> } else {
> httpClient.getHostConfiguration().setHost(host, port);
> }
>
>
After more careful evaluation of the HttpHost constructor (http://grepcode.com/file/repo1.maven.org/maven2/commons-httpclient/commons-httpclient/3.1/org/apache/commons/httpclient/HttpHost.java#HttpHost), I gladly admit I was wrong since as long as the "port" parameter is non-negative it's value will be set into the equivalent instance member; only negative port values would cause the Protocol port value to be used.
Good then!
- Demetrius
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46589
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Demetrius Tsitrelis <dt...@live.com>.
> On June 24, 2014, 11:10 p.m., Demetrius Tsitrelis wrote:
> > The only change I can see regarding "SSL enabling" is to the getProperty() method for the default value. It looks like that change to null wouldn't matter as the constructor for CloudStackApi() would just reassign 8080?
>
> Dmitry Batkovich wrote:
> Maybe I don't understand you but states of properties "cloudAPISSLEnabled" and "cloudAPIPort" are independent. And port value not depends on "cloudAPISSLEnabled" value. (I will assume that you look at not my code)
>
> Demetrius Tsitrelis wrote:
> Sorry Dmitry - I missed a section of code.
>
> Related to the port number changes you made is that now in the constructor for CloudstackClient there is no way for a user to set the port if SSL is used; you have hard-coded port 443.
>
> Dmitry Batkovich wrote:
> No, I didn't ,see https://hc.apache.org/httpclient-3.x/apidocs/org/apache/commons/httpclient/HostConfiguration.html#setHost(java.lang.String, int, org.apache.commons.httpclient.protocol.Protocol)
>
I mean this change:
if (sslEnabled) {
httpClient.getHostConfiguration().setHost(host, port, new Protocol("https", new EasySSLProtocolSocketFactory(), 443));
} else {
httpClient.getHostConfiguration().setHost(host, port);
}
- Demetrius
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46589
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Demetrius Tsitrelis <dt...@live.com>.
> On June 24, 2014, 11:10 p.m., Demetrius Tsitrelis wrote:
> > The only change I can see regarding "SSL enabling" is to the getProperty() method for the default value. It looks like that change to null wouldn't matter as the constructor for CloudStackApi() would just reassign 8080?
>
> Dmitry Batkovich wrote:
> Maybe I don't understand you but states of properties "cloudAPISSLEnabled" and "cloudAPIPort" are independent. And port value not depends on "cloudAPISSLEnabled" value. (I will assume that you look at not my code)
Sorry Dmitry - I missed a section of code.
Related to the port number changes you made is that now in the constructor for CloudstackClient there is no way for a user to set the port if SSL is used; you have hard-coded port 443.
- Demetrius
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46589
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Demetrius Tsitrelis <dt...@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46589
-----------------------------------------------------------
The only change I can see regarding "SSL enabling" is to the getProperty() method for the default value. It looks like that change to null wouldn't matter as the constructor for CloudStackApi() would just reassign 8080?
- Demetrius Tsitrelis
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On July 6, 2014, 9:04 a.m., Demetrius Tsitrelis wrote:
> > _pollTimeoutMs was 600,000 before and now default is 60,000?
ok, it's my mistake
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review47354
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Demetrius Tsitrelis <dt...@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review47354
-----------------------------------------------------------
_pollTimeoutMs was 600,000 before and now default is 60,000?
- Demetrius Tsitrelis
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests
(instead GET), ssl enabling fixed
Posted by Sebastien Goasguen <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review79040
-----------------------------------------------------------
Thank you for submitting your CloudStack contribution through review board. After discussion on the dev@cloudstack.apache.org the community decided to close down review board and start accepting contributiong through GitHub pull requests. We have been using GH PR for several months now and the process is better than review board.
We will keep Review Board open for another week to give you time to migrate your patch to a github PR if you wish. After that time, your patch will no longer be viewable (even though it will not be deleted).
Please consider submitting a pull request.
Great instructions are available at:
https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md
Thank you very much for your time and your contribution to Apache CloudStack, we hope that using this new process will encourage you to do more.
- Sebastien Goasguen
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Demetrius Tsitrelis <dt...@live.com>.
> On July 6, 2014, 8:45 a.m., Demetrius Tsitrelis wrote:
> > Another concern is that the new code is using EasySSLProtocolSocketFactory. Why change Cloudstack to automatically except self-signed certificates here?
>
> Dmitry Batkovich wrote:
> Mm, it seems that you mean "accept" instead "except".
>
> It's not change because in you run current code from repository you will see exception if you use ssl. And you need to insert this code snippet to avoid
> "
> HttpsURLConnection conn = (HttpsURLConnection)url.openConnection();
> conn.setSSLSocketFactory(sslFactory);
> "
>
> Self-signed certificate is simple implementation. If you need other you can add some parameter to constructor and write additional couple lines of code, but in my case self-signed certs only.
Cloudstack should be safe be default. Instead of hard-coding security exceptions and forcing users to recompile the preferred solution is to allow them to choose which certificates to accept. Let them specify that self-signed certificates are trusted by placing them in the trust store.
- Demetrius
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review47353
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On July 6, 2014, 8:45 a.m., Demetrius Tsitrelis wrote:
> > Another concern is that the new code is using EasySSLProtocolSocketFactory. Why change Cloudstack to automatically except self-signed certificates here?
Mm, it seems that you mean "accept" instead "except".
It's not change because in you run current code from repository you will see exception if you use ssl. And you need to insert this code snippet to avoid
"
HttpsURLConnection conn = (HttpsURLConnection)url.openConnection();
conn.setSSLSocketFactory(sslFactory);
"
Self-signed certificate is simple implementation. If you need other you can add some parameter to constructor and write additional couple lines of code, but in my case self-signed certs only.
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review47353
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Demetrius Tsitrelis <dt...@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review47353
-----------------------------------------------------------
Another concern is that the new code is using EasySSLProtocolSocketFactory. Why change Cloudstack to automatically except self-signed certificates here?
- Demetrius Tsitrelis
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On June 24, 2014, 7:28 p.m., Prachi Damle wrote:
> > Hi,
> >
> > 1) EC2Engine.java: I see a lot of new line breaks introduced and its hard to know what the changes are.
> > Is it possible to clean this or provide a list of changes/line numbers changed?
> >
> > 2) CloudStackClient.java:
> > What are the reasons for:
> > - Deletion of JsonAccessor.java and using JsonElement instead
> > - Changes in CloudStackClient::call() method around error handling
> >
> > 3)CloudStackApi.java:
> > - What are the reasons to replace CloudStackCommand by CloudStackQueryBuilder?
> >
Hello
1) I don't see any newlines symbols in EC2Engine.java https://reviews.apache.org/r/21776/diff/#index_header
2) -reasons for deletion JsonAccessor - this class has provided "comfortable" access to json element values, but was non-static class (If you look at code it looks strange that this class is not-static) and used some matching pattern in raw string form (why not some DSL if you really need?). Hence someone who will really needs this functionality has small but unpleasant reduction of performance. And I rewrite this as static class without "matching patterns".
-avoiding "throws Exception" or "catch Exception" in future
3) Ok, I can revert change name if you want
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46567
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Prachi Damle <pr...@citrix.com>.
> On June 24, 2014, 7:28 p.m., Prachi Damle wrote:
> > Hi,
> >
> > 1) EC2Engine.java: I see a lot of new line breaks introduced and its hard to know what the changes are.
> > Is it possible to clean this or provide a list of changes/line numbers changed?
> >
> > 2) CloudStackClient.java:
> > What are the reasons for:
> > - Deletion of JsonAccessor.java and using JsonElement instead
> > - Changes in CloudStackClient::call() method around error handling
> >
> > 3)CloudStackApi.java:
> > - What are the reasons to replace CloudStackCommand by CloudStackQueryBuilder?
> >
>
> Dmitry Batkovich wrote:
> Hello
>
> 1) I don't see any newlines symbols in EC2Engine.java https://reviews.apache.org/r/21776/diff/#index_header
>
> 2) -reasons for deletion JsonAccessor - this class has provided "comfortable" access to json element values, but was non-static class (If you look at code it looks strange that this class is not-static) and used some matching pattern in raw string form (why not some DSL if you really need?). Hence someone who will really needs this functionality has small but unpleasant reduction of performance. And I rewrite this as static class without "matching patterns".
>
> -avoiding "throws Exception" or "catch Exception" in future
>
>
> 3) Ok, I can revert change name if you want
Ok I see the patterns used in JsonAccessor.
Was there any testing with EC2 APIs using any tools done?
Since this patch is changing the mechanism of making the request to CloudStack and handling the response, I think this needs to be tested against CloudStack for variety of EC2 APIs.
- Prachi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46567
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Prachi Damle <pr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review46567
-----------------------------------------------------------
Hi,
1) EC2Engine.java: I see a lot of new line breaks introduced and its hard to know what the changes are.
Is it possible to clean this or provide a list of changes/line numbers changed?
2) CloudStackClient.java:
What are the reasons for:
- Deletion of JsonAccessor.java and using JsonElement instead
- Changes in CloudStackClient::call() method around error handling
3)CloudStackApi.java:
- What are the reasons to replace CloudStackCommand by CloudStackQueryBuilder?
- Prachi Damle
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/
-----------------------------------------------------------
(Updated May 27, 2014, 8:04 p.m.)
Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
Repository: cloudstack-git
Description
-------
* CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
* SSL enabling fixed in EC2Engine.java
continuation of https://reviews.apache.org/r/17586/
Diffs
-----
awsapi/conf/ec2-service.properties.in 82f5ad8
awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
Diff: https://reviews.apache.org/r/21776/diff/
Testing
-------
Thanks,
Dmitry Batkovich
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/
-----------------------------------------------------------
(Updated May 27, 2014, 8:03 p.m.)
Review request for cloudstack, Chiradeep Vittal and daan Hoogland.
Repository: cloudstack-git
Description
-------
* CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
* SSL enabling fixed in EC2Engine.java
continuation of https://reviews.apache.org/r/17586/
Diffs (updated)
-----
awsapi/conf/ec2-service.properties.in 82f5ad8
awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
Diff: https://reviews.apache.org/r/21776/diff/
Testing
-------
Thanks,
Dmitry Batkovich
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On May 27, 2014, 6:55 p.m., Chiradeep Vittal wrote:
> > In EC2engine.java, I still see code blocks such as if (foo) \n blargh() reformatted to if (foo) blargh
> >
> > Please add prachidamle as a reviewer
In new review? https://reviews.apache.org/r/21936/
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review44020
-----------------------------------------------------------
On May 26, 2014, 2:32 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 26, 2014, 2:32 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java 2d8afb5
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackClientException.java ac1b3ef
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java 8b4f267
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On May 27, 2014, 6:55 p.m., Chiradeep Vittal wrote:
> > In EC2engine.java, I still see code blocks such as if (foo) \n blargh() reformatted to if (foo) blargh
> >
> > Please add prachidamle as a reviewer
>
> Dmitry Batkovich wrote:
> In new review? https://reviews.apache.org/r/21936/
>
> Chiradeep Vittal wrote:
> Ah, ok. RB lets you update the old review. It said "updated 4 minutes ago", so I assumed it was an update to the original review request
>
ok, I updated this old review and add reviewer
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review44020
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Chiradeep Vittal <ch...@gmail.com>.
> On May 27, 2014, 6:55 p.m., Chiradeep Vittal wrote:
> > In EC2engine.java, I still see code blocks such as if (foo) \n blargh() reformatted to if (foo) blargh
> >
> > Please add prachidamle as a reviewer
>
> Dmitry Batkovich wrote:
> In new review? https://reviews.apache.org/r/21936/
Ah, ok. RB lets you update the old review. It said "updated 4 minutes ago", so I assumed it was an update to the original review request
- Chiradeep
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review44020
-----------------------------------------------------------
On May 26, 2014, 2:32 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 26, 2014, 2:32 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java 2d8afb5
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackClientException.java ac1b3ef
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java 8b4f267
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Dmitry Batkovich <ba...@gmail.com>.
> On May 27, 2014, 6:55 p.m., Chiradeep Vittal wrote:
> > In EC2engine.java, I still see code blocks such as if (foo) \n blargh() reformatted to if (foo) blargh
> >
> > Please add prachidamle as a reviewer
>
> Dmitry Batkovich wrote:
> In new review? https://reviews.apache.org/r/21936/
>
> Chiradeep Vittal wrote:
> Ah, ok. RB lets you update the old review. It said "updated 4 minutes ago", so I assumed it was an update to the original review request
>
>
> Dmitry Batkovich wrote:
> ok, I updated this old review and add reviewer
Hello anybody, guys any response?=)
- Dmitry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review44020
-----------------------------------------------------------
On May 27, 2014, 8:04 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 27, 2014, 8:04 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, daan Hoogland, and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>
Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead
GET), ssl enabling fixed
Posted by Chiradeep Vittal <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21776/#review44020
-----------------------------------------------------------
In EC2engine.java, I still see code blocks such as if (foo) \n blargh() reformatted to if (foo) blargh
Please add prachidamle as a reviewer
- Chiradeep Vittal
On May 26, 2014, 2:32 p.m., Dmitry Batkovich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21776/
> -----------------------------------------------------------
>
> (Updated May 26, 2014, 2:32 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> * CloudStackClient.java http request mechanism replaced from GET requests to POST for supporting EC2 requests larger than 2KB
> * SSL enabling fixed in EC2Engine.java
>
> continuation of https://reviews.apache.org/r/17586/
>
>
> Diffs
> -----
>
> awsapi/conf/ec2-service.properties.in 82f5ad8
> awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java cd20214
> awsapi/src/com/cloud/bridge/util/JsonAccessor.java 2a94dea
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java PRE-CREATION
> awsapi/src/com/cloud/bridge/util/JsonElementUtil.java 2d8afb5
> awsapi/src/com/cloud/stack/CloudStackApi.java b7a1210
> awsapi/src/com/cloud/stack/CloudStackClient.java 03eba96
> awsapi/src/com/cloud/stack/CloudStackClientException.java PRE-CREATION
> awsapi/src/com/cloud/stack/CloudStackClientException.java ac1b3ef
> awsapi/src/com/cloud/stack/CloudStackCommand.java 8d6aa68
> awsapi/src/com/cloud/stack/CloudStackQueryBuilder.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/CloudStackClientTestCase.java 826cb3a
> awsapi/test/com/cloud/gate/util/JsonAccessorTestCase.java 8603e59
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java PRE-CREATION
> awsapi/test/com/cloud/gate/util/JsonElementUtilTestCase.java 8b4f267
>
> Diff: https://reviews.apache.org/r/21776/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Batkovich
>
>