> On April 16, 2013, 4:31 a.m., Prasanna Santhanam wrote: > > tools/marvin/marvin/cloudstackConnection.py, line 61 > > <https://reviews.apache.org/r/10294/diff/4/?file=281521#file281521line61> > > > > How and when does marvin decide to use POST? Which commands will call > > the POST httpmethod? May be you can provide a more complete example when > > your test is ready.
I saw that make_request() was calling into make_request_with_auth(), so I put in the default value for httpmethod as GET, but missed the fact that make_request_with_auth() is calling itself when retrying - thanks for catching that - I've put in a default value of GET for this parameter. When a unittest calls this function, it would be expected to pass in all arguments in case it wants to use POST, but existing tests that simply use GET don't need to change.. does that look ok? Or is there a better way for me to implement this? > On April 16, 2013, 4:31 a.m., Prasanna Santhanam wrote: > > tools/marvin/marvin/cloudstackConnection.py, line 77 > > <https://reviews.apache.org/r/10294/diff/4/?file=281521#file281521line77> > > > > Are you sure you don't want to urlencode the POST data? > > Yes that is definitely better than doing requestUrl = "&".join(["=".join([r[0], urllib.quote_plus(str(r[1]))]) for r in request]) , which won't take care of spaces either.. have made the change here and even in make_request_without_auth().. > On April 16, 2013, 4:31 a.m., Prasanna Santhanam wrote: > > tools/marvin/marvin/cloudstackConnection.py, line 79 > > <https://reviews.apache.org/r/10294/diff/4/?file=281521#file281521line79> > > > > Minor nitpick: urllib2.Request() can be used for both GET and POST and > > you can set the data argument only when you intend to use POST. That will > > reduce some code duplication here. I changed both cases to use urllib2.Request(), but still it's still the same number of lines :) Is there a way I can merge those two separate calls to urllib2.Request(url) and urllib2.Request(url, requestUrl) into a single call? - Venkata Siva Vijayendra ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10294/#review19245 ----------------------------------------------------------- On April 16, 2013, 2:53 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 16, 2013, 2:53 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 d963b74 > api/src/org/apache/cloudstack/api/BaseCmd.java 42c0680 > api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 77ba9fe > 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 d281e5b > server/test/com/cloud/vm/MockUserVmManagerImpl.java fd826d9 > 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 fb760bf > tools/marvin/marvin/cloudstackConnection.py 1caeef3 > > Diff: https://reviews.apache.org/r/10294/diff/ > > > Testing > ------- > > > Thanks, > > Venkata Siva Vijayendra Bhamidipati > >
