Also - you need to run a RAT test before the merge (if and when it happens). The test file doesn't have a license header for ASF.
-- Prasanna., On Wed, Apr 24, 2013 at 12:13:33AM +0530, Prasanna Santhanam wrote: > Vijay - Min applied the patch in the branch http_post. I made changes > to your test to make it use libraries from marvin appropriately. Don't > hesitate to go change marvin if it makes sense to next time. > > What I'm seeing is that in your examples you've attached - you send > all the request for deployVMCmd as POST whereas I imagine the user > would only want to send his userdata as POST and the rest of the > arguments to deployVM as GET. Is this not the case? > > I'm doing : > deployVM [ GET(template = builtin, service = Small, zone = myzone) and POST( > userdata = 2ksizeData) ] > > while you're doing > > deployVM [ POST(template = builtin, service = Small, zone = myzone, userdata > = 2kSizeData) ] > > It's not clear how this POST data should go from the spec. Can you > please clarify? Is it sensible to make either style work? Do we flout > any RFC? > > Thanks, > > -- > Prasanna., > > On Tue, Apr 23, 2013 at 04:08:14AM -0000, Prasanna Santhanam wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/10294/#review19574 > > ----------------------------------------------------------- > > > > > > The patch doesn't apply for me: > > > > ~/workspace/cloudstack/incubator-cloudstack/patch(branch:master) ?? git am > > -s 10294.patch > > > > Applying: CLOUDSTACK-1086: DeployVirtualMachine userdata enhancements > > error: patch failed: > > api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java:312 > > error: api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java: > > patch does not apply > > error: patch failed: server/test/com/cloud/vm/dao/UserVmDaoImplTest.java:20 > > error: server/test/com/cloud/vm/dao/UserVmDaoImplTest.java: patch does not > > apply > > error: patch failed: setup/db/db/schema-410to420.sql:777 > > error: setup/db/db/schema-410to420.sql: patch does not apply > > /Users/tsp/workspace/cloudstack/incubator-cloudstack/.git/rebase-apply/patch:1153: > > new blank line at EOF. > > + > > Patch failed at 0001 CLOUDSTACK-1086: DeployVirtualMachine userdata > > enhancements > > When you have resolved this problem run "git am --resolved". > > If you would prefer to skip this patch, instead run "git am --skip". > > To restore the original branch and stop patching run "git am --abort". > > > > I'm trying to run your marvin test and see what the issue is in sending > > through post data. All commands now take postdata as a parameter. > > > > - Prasanna Santhanam > > > > > > On April 23, 2013, 1:57 a.m., Venkata Siva Vijayendra Bhamidipati wrote: > > > > > > ----------------------------------------------------------- > > > This is an automatically generated e-mail. To reply, visit: > > > https://reviews.apache.org/r/10294/ > > > ----------------------------------------------------------- > > > > > > (Updated April 23, 2013, 1:57 a.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 aa21136 > > > api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 > > > api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java > > > 70c0159 > > > api/src/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java > > > ff8fff1 > > > 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 1843f60 > > > server/test/com/cloud/vm/MockUserVmManagerImpl.java 0d0a8f4 > > > 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 10cdbba > > > test/integration/component/test_deploy_vm_with_userdata.py PRE-CREATION > > > > > > Diff: https://reviews.apache.org/r/10294/diff/ > > > > > > > > > Testing > > > ------- > > > > > > Manual testing done with scripts generating large/small userdata during > > > creation of VMs, with both GET and POST requests confirmed to work > > > correctly, on both integration port and the 8080 port. > > > > > > Basic Marvin test to create a VM with user data has been written. Since > > > marvin doesn't yet support POSTable APIs, as soon as that is made > > > available for create/update VM operations, marvin tests will be written > > > for the same. Requesting that this be noted as an AI for the future. > > > > > > > > > Thanks, > > > > > > Venkata Siva Vijayendra Bhamidipati > > > > > > > > > > > ------------------------ > Powered by BigRock.com ------------------------ Powered by BigRock.com
