You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Nitin Mehta <Ni...@citrix.com> on 2014/05/30 20:14:56 UTC

[ACS44] Cherry pick CLOUDSTACK-6599

Hello Daan,

Can you please cherry-pick the following commit from 4.4-forward to 4.4 ?

commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
    CLOUDSTACK-6599:
    1. Adding the missing Template/Volume URLs expiration functionality
    2. Improvement - While deleting the volume during expiration use rm -rf as vmware now contains directoy
    3. Improvement - Use standard Answer so that the error gets logged in case deletion of expiration link didnt work fine.
    4. Improvement - In case of domain change, expire the old urls

Thanks,
-Nitin

Re: [ACS44] Cherry pick CLOUDSTACK-6599

Posted by Nitin Mehta <Ni...@citrix.com>.
Thanks Daan. Regarding test cases, I will check and get back.
Just that we don¹t loose this it in mails I have created a bug for 4.4,
for adding test cases if they don¹t already exist.

-Nitin

On 03/06/14 1:08 AM, "Daan Hoogland" <da...@gmail.com> wrote:

>strong point,
>
>I added both commits. Are there tests to the old code as well? this is
>a lot of untested code like this.
>
>On Mon, Jun 2, 2014 at 7:50 PM, Nitin Mehta <Ni...@citrix.com>
>wrote:
>> Daan - Thanks for your comment. The java upgrade code has never been
>>data
>> migration code only.
>> I have always seen complex DDL logic being handled by java upgrade code
>> because we can write complex logic and catch exceptions gracefully. As
>>you
>> see from the links below - add column if it doesn't exist logic is not
>> trivial and hence its been added in java upgrade path. I have infact
>> borrowed the logic from upgrade410-420 java code. Therefore, you can't
>> figure out the schema through the schema sql files.
>>
>> Thanks,
>> -Nitin
>>
>> On 31/05/14 2:19 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>>
>>>now your are changing schema in java code! please don't do that. Those
>>>are for data migration. If we start 5.0 we want to be able read the
>>>sql to find the actual schema.
>>>
>>>http://stackoverflow.com/questions/972922/add-column-to-mysql-table-if-i
>>>t-
>>>does-not-exist
>>>http://stackoverflow.com/questions/14381895/mysql-add-column-if-not-exis
>>>t
>>>
>>>
>>>On Sat, May 31, 2014 at 2:05 AM, Nitin Mehta <Ni...@citrix.com>
>>>wrote:
>>>> Please cherry-pick be765ce8680564b743a73dd360c590c0e495c204 as well as
>>>> part of this bug.
>>>> One more thing to add, majority of code is for the functionality
>>>>which I
>>>> found missing in 4.4 and found some bugs which I termed as
>>>>improvements
>>>> over previous design.
>>>>
>>>> Thanks,
>>>> -Nitin
>>>>
>>>> On 30/05/14 3:06 PM, "Nitin Mehta" <Ni...@citrix.com> wrote:
>>>>
>>>>>Daan - Here improvements are actually bug fixes that should be fixed.
>>>>>
>>>>>Thanks,
>>>>>-Nitin
>>>>>
>>>>>On 30/05/14 1:47 PM, "Daan Hoogland" <da...@gmail.com> wrote:
>>>>>
>>>>>>That's a lot of improvements without tests, Nitin.
>>>>>>
>>>>>>On Fri, May 30, 2014 at 8:14 PM, Nitin Mehta <Ni...@citrix.com>
>>>>>>wrote:
>>>>>>> Hello Daan,
>>>>>>>
>>>>>>> Can you please cherry-pick the following commit from 4.4-forward to
>>>>>>>4.4
>>>>>>>?
>>>>>>>
>>>>>>> commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
>>>>>>>     CLOUDSTACK-6599:
>>>>>>>     1. Adding the missing Template/Volume URLs expiration
>>>>>>>functionality
>>>>>>>     2. Improvement - While deleting the volume during expiration
>>>>>>>use
>>>>>>>rm
>>>>>>>-rf
>>>>>>> as vmware now contains directoy
>>>>>>>     3. Improvement - Use standard Answer so that the error gets
>>>>>>>logged
>>>>>>>in
>>>>>>> case deletion of expiration link didnt work fine.
>>>>>>>     4. Improvement - In case of domain change, expire the old urls
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -Nitin
>>>>>>
>>>>>>
>>>>>>
>>>>>>--
>>>>>>Daan
>>>>>
>>>>
>>>
>>>
>>>
>>>--
>>>Daan
>>
>
>
>
>-- 
>Daan


Re: [ACS44] Cherry pick CLOUDSTACK-6599

Posted by Daan Hoogland <da...@gmail.com>.
strong point,

I added both commits. Are there tests to the old code as well? this is
a lot of untested code like this.

On Mon, Jun 2, 2014 at 7:50 PM, Nitin Mehta <Ni...@citrix.com> wrote:
> Daan - Thanks for your comment. The java upgrade code has never been data
> migration code only.
> I have always seen complex DDL logic being handled by java upgrade code
> because we can write complex logic and catch exceptions gracefully. As you
> see from the links below - add column if it doesn't exist logic is not
> trivial and hence its been added in java upgrade path. I have infact
> borrowed the logic from upgrade410-420 java code. Therefore, you can't
> figure out the schema through the schema sql files.
>
> Thanks,
> -Nitin
>
> On 31/05/14 2:19 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>
>>now your are changing schema in java code! please don't do that. Those
>>are for data migration. If we start 5.0 we want to be able read the
>>sql to find the actual schema.
>>
>>http://stackoverflow.com/questions/972922/add-column-to-mysql-table-if-it-
>>does-not-exist
>>http://stackoverflow.com/questions/14381895/mysql-add-column-if-not-exist
>>
>>
>>On Sat, May 31, 2014 at 2:05 AM, Nitin Mehta <Ni...@citrix.com>
>>wrote:
>>> Please cherry-pick be765ce8680564b743a73dd360c590c0e495c204 as well as
>>> part of this bug.
>>> One more thing to add, majority of code is for the functionality which I
>>> found missing in 4.4 and found some bugs which I termed as improvements
>>> over previous design.
>>>
>>> Thanks,
>>> -Nitin
>>>
>>> On 30/05/14 3:06 PM, "Nitin Mehta" <Ni...@citrix.com> wrote:
>>>
>>>>Daan - Here improvements are actually bug fixes that should be fixed.
>>>>
>>>>Thanks,
>>>>-Nitin
>>>>
>>>>On 30/05/14 1:47 PM, "Daan Hoogland" <da...@gmail.com> wrote:
>>>>
>>>>>That's a lot of improvements without tests, Nitin.
>>>>>
>>>>>On Fri, May 30, 2014 at 8:14 PM, Nitin Mehta <Ni...@citrix.com>
>>>>>wrote:
>>>>>> Hello Daan,
>>>>>>
>>>>>> Can you please cherry-pick the following commit from 4.4-forward to
>>>>>>4.4
>>>>>>?
>>>>>>
>>>>>> commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
>>>>>>     CLOUDSTACK-6599:
>>>>>>     1. Adding the missing Template/Volume URLs expiration
>>>>>>functionality
>>>>>>     2. Improvement - While deleting the volume during expiration use
>>>>>>rm
>>>>>>-rf
>>>>>> as vmware now contains directoy
>>>>>>     3. Improvement - Use standard Answer so that the error gets
>>>>>>logged
>>>>>>in
>>>>>> case deletion of expiration link didnt work fine.
>>>>>>     4. Improvement - In case of domain change, expire the old urls
>>>>>>
>>>>>> Thanks,
>>>>>> -Nitin
>>>>>
>>>>>
>>>>>
>>>>>--
>>>>>Daan
>>>>
>>>
>>
>>
>>
>>--
>>Daan
>



-- 
Daan

Re: [ACS44] Cherry pick CLOUDSTACK-6599

Posted by Nitin Mehta <Ni...@citrix.com>.
Daan - Thanks for your comment. The java upgrade code has never been data
migration code only.
I have always seen complex DDL logic being handled by java upgrade code
because we can write complex logic and catch exceptions gracefully. As you
see from the links below - add column if it doesn't exist logic is not
trivial and hence its been added in java upgrade path. I have infact
borrowed the logic from upgrade410-420 java code. Therefore, you can't
figure out the schema through the schema sql files.

Thanks,
-Nitin

On 31/05/14 2:19 AM, "Daan Hoogland" <da...@gmail.com> wrote:

>now your are changing schema in java code! please don't do that. Those
>are for data migration. If we start 5.0 we want to be able read the
>sql to find the actual schema.
>
>http://stackoverflow.com/questions/972922/add-column-to-mysql-table-if-it-
>does-not-exist
>http://stackoverflow.com/questions/14381895/mysql-add-column-if-not-exist
>
>
>On Sat, May 31, 2014 at 2:05 AM, Nitin Mehta <Ni...@citrix.com>
>wrote:
>> Please cherry-pick be765ce8680564b743a73dd360c590c0e495c204 as well as
>> part of this bug.
>> One more thing to add, majority of code is for the functionality which I
>> found missing in 4.4 and found some bugs which I termed as improvements
>> over previous design.
>>
>> Thanks,
>> -Nitin
>>
>> On 30/05/14 3:06 PM, "Nitin Mehta" <Ni...@citrix.com> wrote:
>>
>>>Daan - Here improvements are actually bug fixes that should be fixed.
>>>
>>>Thanks,
>>>-Nitin
>>>
>>>On 30/05/14 1:47 PM, "Daan Hoogland" <da...@gmail.com> wrote:
>>>
>>>>That's a lot of improvements without tests, Nitin.
>>>>
>>>>On Fri, May 30, 2014 at 8:14 PM, Nitin Mehta <Ni...@citrix.com>
>>>>wrote:
>>>>> Hello Daan,
>>>>>
>>>>> Can you please cherry-pick the following commit from 4.4-forward to
>>>>>4.4
>>>>>?
>>>>>
>>>>> commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
>>>>>     CLOUDSTACK-6599:
>>>>>     1. Adding the missing Template/Volume URLs expiration
>>>>>functionality
>>>>>     2. Improvement - While deleting the volume during expiration use
>>>>>rm
>>>>>-rf
>>>>> as vmware now contains directoy
>>>>>     3. Improvement - Use standard Answer so that the error gets
>>>>>logged
>>>>>in
>>>>> case deletion of expiration link didnt work fine.
>>>>>     4. Improvement - In case of domain change, expire the old urls
>>>>>
>>>>> Thanks,
>>>>> -Nitin
>>>>
>>>>
>>>>
>>>>--
>>>>Daan
>>>
>>
>
>
>
>-- 
>Daan


Re: [ACS44] Cherry pick CLOUDSTACK-6599

Posted by Daan Hoogland <da...@gmail.com>.
now your are changing schema in java code! please don't do that. Those
are for data migration. If we start 5.0 we want to be able read the
sql to find the actual schema.

http://stackoverflow.com/questions/972922/add-column-to-mysql-table-if-it-does-not-exist
http://stackoverflow.com/questions/14381895/mysql-add-column-if-not-exist


On Sat, May 31, 2014 at 2:05 AM, Nitin Mehta <Ni...@citrix.com> wrote:
> Please cherry-pick be765ce8680564b743a73dd360c590c0e495c204 as well as
> part of this bug.
> One more thing to add, majority of code is for the functionality which I
> found missing in 4.4 and found some bugs which I termed as improvements
> over previous design.
>
> Thanks,
> -Nitin
>
> On 30/05/14 3:06 PM, "Nitin Mehta" <Ni...@citrix.com> wrote:
>
>>Daan - Here improvements are actually bug fixes that should be fixed.
>>
>>Thanks,
>>-Nitin
>>
>>On 30/05/14 1:47 PM, "Daan Hoogland" <da...@gmail.com> wrote:
>>
>>>That's a lot of improvements without tests, Nitin.
>>>
>>>On Fri, May 30, 2014 at 8:14 PM, Nitin Mehta <Ni...@citrix.com>
>>>wrote:
>>>> Hello Daan,
>>>>
>>>> Can you please cherry-pick the following commit from 4.4-forward to 4.4
>>>>?
>>>>
>>>> commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
>>>>     CLOUDSTACK-6599:
>>>>     1. Adding the missing Template/Volume URLs expiration functionality
>>>>     2. Improvement - While deleting the volume during expiration use rm
>>>>-rf
>>>> as vmware now contains directoy
>>>>     3. Improvement - Use standard Answer so that the error gets logged
>>>>in
>>>> case deletion of expiration link didnt work fine.
>>>>     4. Improvement - In case of domain change, expire the old urls
>>>>
>>>> Thanks,
>>>> -Nitin
>>>
>>>
>>>
>>>--
>>>Daan
>>
>



-- 
Daan

Re: [ACS44] Cherry pick CLOUDSTACK-6599

Posted by Nitin Mehta <Ni...@citrix.com>.
Please cherry-pick be765ce8680564b743a73dd360c590c0e495c204 as well as
part of this bug.
One more thing to add, majority of code is for the functionality which I
found missing in 4.4 and found some bugs which I termed as improvements
over previous design.

Thanks,
-Nitin

On 30/05/14 3:06 PM, "Nitin Mehta" <Ni...@citrix.com> wrote:

>Daan - Here improvements are actually bug fixes that should be fixed.
>
>Thanks,
>-Nitin
>
>On 30/05/14 1:47 PM, "Daan Hoogland" <da...@gmail.com> wrote:
>
>>That's a lot of improvements without tests, Nitin.
>>
>>On Fri, May 30, 2014 at 8:14 PM, Nitin Mehta <Ni...@citrix.com>
>>wrote:
>>> Hello Daan,
>>>
>>> Can you please cherry-pick the following commit from 4.4-forward to 4.4
>>>?
>>>
>>> commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
>>>     CLOUDSTACK-6599:
>>>     1. Adding the missing Template/Volume URLs expiration functionality
>>>     2. Improvement - While deleting the volume during expiration use rm
>>>-rf
>>> as vmware now contains directoy
>>>     3. Improvement - Use standard Answer so that the error gets logged
>>>in
>>> case deletion of expiration link didnt work fine.
>>>     4. Improvement - In case of domain change, expire the old urls
>>>
>>> Thanks,
>>> -Nitin
>>
>>
>>
>>-- 
>>Daan
>


Re: [ACS44] Cherry pick CLOUDSTACK-6599

Posted by Nitin Mehta <Ni...@citrix.com>.
Daan - Here improvements are actually bug fixes that should be fixed.

Thanks,
-Nitin

On 30/05/14 1:47 PM, "Daan Hoogland" <da...@gmail.com> wrote:

>That's a lot of improvements without tests, Nitin.
>
>On Fri, May 30, 2014 at 8:14 PM, Nitin Mehta <Ni...@citrix.com>
>wrote:
>> Hello Daan,
>>
>> Can you please cherry-pick the following commit from 4.4-forward to 4.4
>>?
>>
>> commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
>>     CLOUDSTACK-6599:
>>     1. Adding the missing Template/Volume URLs expiration functionality
>>     2. Improvement - While deleting the volume during expiration use rm
>>-rf
>> as vmware now contains directoy
>>     3. Improvement - Use standard Answer so that the error gets logged
>>in
>> case deletion of expiration link didnt work fine.
>>     4. Improvement - In case of domain change, expire the old urls
>>
>> Thanks,
>> -Nitin
>
>
>
>-- 
>Daan


Re: [ACS44] Cherry pick CLOUDSTACK-6599

Posted by Daan Hoogland <da...@gmail.com>.
That's a lot of improvements without tests, Nitin.

On Fri, May 30, 2014 at 8:14 PM, Nitin Mehta <Ni...@citrix.com> wrote:
> Hello Daan,
>
> Can you please cherry-pick the following commit from 4.4-forward to 4.4 ?
>
> commit 48ea9e0b5e87fee067b711890cd5a5d7c9079bf1
>     CLOUDSTACK-6599:
>     1. Adding the missing Template/Volume URLs expiration functionality
>     2. Improvement - While deleting the volume during expiration use rm -rf
> as vmware now contains directoy
>     3. Improvement - Use standard Answer so that the error gets logged in
> case deletion of expiration link didnt work fine.
>     4. Improvement - In case of domain change, expire the old urls
>
> Thanks,
> -Nitin



-- 
Daan