incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Wido den Hollander <w...@widodh.nl>
Subject Re: [MERGE]Storage refactor branch
Date Fri, 15 Feb 2013 19:51:17 GMT


On 02/15/2013 08:03 PM, Edison Su wrote:
>
>
>> -----Original Message-----
>> From: Hugo Trippaers [mailto:HTrippaers@schubergphilis.com]
>> Sent: Friday, February 15, 2013 6:39 AM
>> To: cloudstack-dev@incubator.apache.org
>> Cc: John Burwell <jburwell@basho.com> (jburwell@basho.com); Mike
>> Tutkowski (mike.tutkowski@solidfire.com)
>> Subject: RE: [MERGE]Storage refactor branch
>>
>> Hey Edison,
>>
>> Thanks for the update
>>
>>> -----Original Message-----
>>> From: Edison Su [mailto:Edison.su@citrix.com]
>>> Sent: Friday, February 15, 2013 12:13 AM
>>> To: cloudstack-dev@incubator.apache.org
>>> Cc: John Burwell <jburwell@basho.com> (jburwell@basho.com); Mike
>>> Tutkowski (mike.tutkowski@solidfire.com)
>>> Subject: [MERGE]Storage refactor branch
>>>
>>> Hi All,
>>>       Here is the update from storage refactor branch since last week:
>>>
>>> 1.       clean/hook up snapshot into new storage framework, separate taking
>>> snapshot and backup snapshot. Add a snapshotstrategy interface for
>>> people want to change or replace how snapshot is processed in
>>> cloudstack. Current snapshot code is little bit hacky, welcome to replace it
>> for your need.
>>>        Since last week I created storage_refactor branch, seems no
>>> objection for what I am doing, so I want to merge the branch into
>>> master, by end of this week, due to the following reasons:
>>>           Maintain such big patch(more than 20K) outside of master,
>>> touch all the storage code, is not an easy task.
>>>           After merge into master, other developers who developing
>>> storage features can work on master directly, thus I don't need to
>>> rebase the branch regularly.
>>>        About test, I only tested basic features, like, stop/start vm,
>>> attach/detach volumes, take snapshots in devcloud. For sure, I'll
>>> break something. The more test from other people, will help to me to
>>> stabilize the code. About unit test, I have some unit test and
>>> integration test for devcloud, but both of them needs database and
>>> devcloud environment, so they are disabled by default and unit test
>>> themselves are broken also. I'll write more tests during the stabilization.
>>
>> I understand the need to merge into master, it is a serious pain to keep
>> updating the branch with that latest state of master. However this testing
>
> Thanks for understand the painful of rebaseing such big patch. There is another reason
I want to merge this branch into master ASAP: all the new storage features are depended this
branch: like zone-wide storage, make nfs secondary storage as optional, re-write s3/swift
integration etc. Let's get it merged, adding more awesome features:)
>

I'm waiting on starting with the new RBD features until this is merged in.

I also understand what a hassle this is, got the same when developing 
the original RBD without having commit rights.

Wido

>> concerns me a lot, we all agreed that we would focus on automated testing
>> both on the unittest level and on the functional level. This is the first merge
>> request since we had the last discussion on testing (related to merging the
>> javelin and other branches). The clear consensus was that commits would
>> have to be accompanied by unittests (at least for the changed pieces of code)
>> and preferable some automated functional test.
>>
>> Considering that, I think that a merge is not the right this to do at the
>> moment. Instead of merging into master and then fixing stuff, we should
>> focus on adding testing to this branch so we can merge a well-tested unit
>> later. Notice my use of 'we' here, we all said focus on testing, so we all should
>> help out. Unfortunately I'm pretty busy with packaging and the $dayjob, but
>> if you have something I can help you with regarding setting up unittests feel
>> free to let me know. You already mentioned that you have some, but the
>> databases need to be mocked?
>>
>> It would be great if you could spend some time to details what tests you
>> have in place, so we can fix those. With the help of the cobertura report we
>> can figure out where the risks are. Prassana and myself are also working on
>> the automated test framework, so if you have some marvin tests, we can
>> help to automate them.
>
> My branch is not a feature branch, while other features are depended on it. I didn't
add any new feature on the branch, all the existing marvin automated tests should work. Instead
of testing and fixing on my branch then merge, is it better to test and fix on master after
the merge, using existing marvin test?
> Again, the rebase to master is painful for such branch.
>
>
>>
>> Anybody else willing to help put in some tests into the storage_refactor
>> branch?
>>
>> Cheers,
>>
>> Hugo

Mime
View raw message