Hi Prasanna, Thanks for catching the absence of license in the new file - I'll make the required changes.
Regarding the POST, the whole query needs to be sent in as a POST query and not only the userdata as POST data, because CS APIs are supposed to work with either GET or POST. The doGet() and doPost() httpservlet routines in the mgmt. server's ApiServlet.java will process them in the appropriate way. The integration port code path also calls into the same code path that the latter routines call into. Regards, Vijay -----Original Message----- From: Prasanna Santhanam [mailto:t...@apache.org] Sent: Tuesday, April 23, 2013 11:51 AM To: Vijayendra Bhamidipati; Min Chen; Chip Childers; CloudStack Dev Subject: Re: Review Request: Remove 2k limitation for user data on a deployVMCmd issued as an HTTP POST request 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:3 > > 12 > > 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
