Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (nike.apache.org: domain of Murali.Reddy@citrix.com
 designates 203.166.19.134 as permitted sender)
From: Murali Reddy <Murali.Reddy@citrix.com>
To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, Ryan Dietrich
	<ryan@betterservers.com>, Marcus Sorensen <shadowsor@gmail.com>
CC: cloudstack <cloudstack-dev@incubator.apache.org>
Subject: Re: Review Request 12752: Async commands can inject the job id
Thread-Topic: Review Request 12752: Async commands can inject the job id
Thread-Index: AQHOhA5+Wfnq8uR+LUiE8VzoUc1H1ZlqkIoAgAAQOQCAAPBzAA==
Date: Fri, 19 Jul 2013 09:23:27 +0000
Message-ID: <CE0EFE1B.44D8A%murali.reddy@citrix.com>
In-Reply-To: <20130719003248.1153.23435@reviews.apache.org>
Accept-Language: en-US
Content-Language: en-US
user-agent: Microsoft-MacOutlook/14.2.5.121010
Content-Type: text/plain; charset="us-ascii"
Content-ID: <BA2562495199AB408E7CABA76CFB7F19@citrix.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Ryan,

You should open a bug and add details on what the use case you are trying
to solve. Since CloudStack is not in control of UUID generation, you
should at least check that uniqueness of passed in UUID.

Also please introduce a new smoke test for 'injected job id' instead of
piggy backing the test for deploy VM.

On 19/07/13 6:02 AM, "Ryan Dietrich" <ryan@betterservers.com> wrote:

>
>
>> On July 18, 2013, 11:34 p.m., Marcus Sorensen wrote:
>> > server/src/com/cloud/async/AsyncJobVO.java, line 141
>> >=20
>><https://reviews.apache.org/r/12752/diff/1/?file=3D323558#file323558line1=
41
>>>
>> >
>> >     I'm not immediately certain why this was pulled out.
>
>Nothing is using it.  I did a scan to see if anyone is using it, and I
>didn't see anything.  Dead could should be removed, right?  Git will
>remember for us if we need it later.
>
>
>- Ryan
>
>
>-----------------------------------------------------------
>This is an automatically generated e-mail. To reply, visit:
>https://reviews.apache.org/r/12752/#review23457
>-----------------------------------------------------------
>
>
>On July 18, 2013, 11:28 p.m., Ryan Dietrich wrote:
>>=20
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/12752/
>> -----------------------------------------------------------
>>=20
>> (Updated July 18, 2013, 11:28 p.m.)
>>=20
>>=20
>> Review request for cloudstack and Marcus Sorensen.
>>=20
>>=20
>> Repository: cloudstack-git
>>=20
>>=20
>> Description
>> -------
>>=20
>> I added "injectedjobid" to the BaseAsyncCmd class as a parameter.
>> If set, it will allow you to tell Cloudstack what the job id instead of
>>it choosing one.
>> A basic string length test is done to verify the variable passed in is
>>actually a UUID.
>> If it is not valid, it is ignored and the job generates it's own.
>>=20
>>=20
>> Diffs
>> -----
>>=20
>>   api/src/org/apache/cloudstack/api/BaseAsyncCmd.java 0e6f95d
>>   server/src/com/cloud/api/ApiServer.java 95f17af
>>   server/src/com/cloud/async/AsyncJobVO.java 41eccb4
>>   test/integration/smoke/test_deploy_vm.py 425aeb7
>>   tools/marvin/marvin/codegenerator.py 632b8c6
>>   tools/marvin/marvin/integration/lib/base.py 161d03c
>>=20
>> Diff: https://reviews.apache.org/r/12752/diff/
>>=20
>>=20
>> Testing
>> -------
>>=20
>> Updated marvin, updated the deploy vm test.  Ran multiple async
>>commands manually, with and without injectedjobid present, no issues
>>detected.
>>=20
>>=20
>> Thanks,
>>=20
>> Ryan Dietrich
>>=20
>>
>
>