> On April 11, 2013, 8:54 p.m., Min Chen wrote:
> > server/src/com/cloud/vm/UserVmManagerImpl.java, line 2581
> > <https://reviews.apache.org/r/10294/diff/3/?file=280319#file280319line2581>
> >
> >     Based on your comment, for GET, maxBytes = 4K, for POST, maxBytes = 
> > 32K. Why not define these constants instead of an intermediate 
> > MAX_USER_DATA_LENGTH_BYTES and do some math calculation here? This code is 
> > hard to maintain later in case that http length standard changes later.

I just used the existing constant value, but yes, defining explicit constants 
for userdata length for different http methods will be cleaner. Will do that.


> On April 11, 2013, 8:54 p.m., Min Chen wrote:
> > server/test/com/cloud/vm/dao/UserVmDaoImplTest.java, line 61
> > <https://reviews.apache.org/r/10294/diff/3/?file=280321#file280321line61>
> >
> >     This test only tests database persistence. We may need another 
> > automated test to issue a POST request.

Will add a marvin test for that.


> On April 11, 2013, 8:54 p.m., Min Chen wrote:
> > server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java, line 1
> > <https://reviews.apache.org/r/10294/diff/3/?file=280322#file280322line1>
> >
> >     Where are UserVmDaoTestConfiguration used in your test? You are missing 
> > some Spring Junit annotation in your UserVmDaoImplTest.

Thanks for catching this - looks like the Testconfiguration.java and 
textcontext.xml files slipped out of the diffs - when uploading the new diffs, 
I'll ensure that they're present.


- Venkata Siva Vijayendra


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10294/#review19030
-----------------------------------------------------------


On April 10, 2013, 9:59 p.m., Venkata Siva Vijayendra Bhamidipati wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10294/
> -----------------------------------------------------------
> 
> (Updated April 10, 2013, 9:59 p.m.)
> 
> 
> Review request for cloudstack, Chip Childers, Hugo Trippaers, Kelven Yang, 
> and Min Chen.
> 
> 
> Description
> -------
> 
> Please refer to 
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/DeployVirtualMachine+userdata+enhancements
>  for a background on the requirements driving this patch.
> 
> This patch hasn't been extensively tested yet, and I will update this request 
> with more info. I am uploading a first diff for initial review/comments.
> 
> 
> This addresses bug CLOUDSTACK-1086.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/vm/UserVmService.java 2c33d41 
>   api/src/org/apache/cloudstack/api/BaseCmd.java 8fef422 
>   api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 21a45f8 
>   core/src/com/cloud/vm/UserVmVO.java a16eaf9 
>   server/src/com/cloud/api/ApiDispatcher.java 925d90a 
>   server/src/com/cloud/api/ApiServer.java d842819 
>   server/src/com/cloud/api/ApiServerService.java 12d8b52 
>   server/src/com/cloud/api/ApiServlet.java 03bfb5f 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 1c3764a 
>   server/test/com/cloud/vm/MockUserVmManagerImpl.java dd8dd83 
>   server/test/com/cloud/vm/dao/UserVmDaoImplTest.java 0936180 
>   server/test/com/cloud/vm/dao/UserVmDaoTestConfiguration.java PRE-CREATION 
>   server/test/resources/UserVMDaoTestContext.xml PRE-CREATION 
>   setup/db/db/schema-410to420.sql c7c8b5b 
> 
> Diff: https://reviews.apache.org/r/10294/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Venkata Siva Vijayendra Bhamidipati
> 
>

Reply via email to