Re: Review Request 21776: AWSAPI: cloudstack api on POST requests (instead GET), ssl enabling fixed

[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
> 
>