Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: cloudstack-dev@incubator.apache.org
Received-SPF: neutral (athena.apache.org: local policy)
References: 
 <0BCCCE152323764BB7FD6AE6D7A1D9060102894ECFFE@BANPMAILBOX01.citrite.net>
 <-2095990210157392433@unknownmsgid>
 <B1DF26ECC0458748AC97CECE2DA98D41012FA385E34E@SJCPMAILBOX01.citrite.net>
From: Chip Childers <chip.childers@sungard.com>
Mime-Version: 1.0 (1.0)
In-Reply-To: 
 <B1DF26ECC0458748AC97CECE2DA98D41012FA385E34E@SJCPMAILBOX01.citrite.net>
Date: Sat, 22 Dec 2012 14:12:57 -0500
Message-ID: <-8427102135254277042@unknownmsgid>
Subject: Re: Merge request: Merging api_refactoring on master
To: "<cloudstack-dev@incubator.apache.org>"
 <cloudstack-dev@incubator.apache.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

Alex/Rohit,

Great to hear that I misunderstood! If it is supposed to be compatible
as a goal, then my concern can be switched to a question of how are we
doing compatibility testing.

- chip

Sent from my iPhone.

On Dec 22, 2012, at 1:52 PM, Alex Huang <Alex.Huang@citrix.com> wrote:

> Correct me if I'm wrong here, Rohit.  I believe the refactoring work cons=
ists of the following.
>
> - Moving java packages around for better grouping.  These doesn't have mu=
ch impact on the query API, except for maybe some typos in the commands.pro=
perties file.
> - Splitting the commands that have optional admin commands into an admin =
package.  The current commands.properties should still be referencing the a=
dmin package as it is backwards compatible with 4.0.0.
> - Additions in the processing engine to process the new annotations added=
.  If the annotation is not there, the processing remains the same as the 4=
.0.0.
> - Work on the response side to make sure the UUIDs that were being return=
ed are not done through n+1 queries but from a big join.
>
> The work on uuid etc actually happened in 3.0.0 but it was done in rather=
 horrific fashion, causing problems in upgrade, performance, scalability, a=
nd security.  We're really just cleaning up in terms of that.  If you're ru=
nning 3.0-4.0, you should be seeing uuids in the responses and using uuids =
in the incoming query parameters already.  If you see specific examples whe=
re it is not, it's a bug in the api.
>
> I don't think it will break the end user api other than bugs introduced d=
uring coding.  In fact, we took great effort to keep the api the same.  If =
we didn't have that constraint, I would  have designed a completely new RES=
T style api instead of keeping the semi-awkward query language the current =
api is on.  This refactoring is all about keeping the over-the-wire api the=
 same while moving a lot of the hard-coded parameter checking, security che=
cking, etc into adapters to decouple the different aspects of checking to s=
ee if an api should be executed.
>
> --Alex
>
>> -----Original Message-----
>> From: Chip Childers [mailto:chip.childers@sungard.com]
>> Sent: Saturday, December 22, 2012 6:26 AM
>> To: <cloudstack-dev@incubator.apache.org>
>> Subject: Re: Merge request: Merging api_refactoring on master
>>
>> So it sounds like a ton of good work.
>>
>> However, the proposed merge also sounds like it breaks public API
>> compatibility with 4.0.0 in both the uuid / id changes and in the list
>> result changes.
>>
>> So I guess this is my first question: does the community agree that
>> the benefits of these changes outweigh the concerns about moving
>> straight from 4.0.0 to 5.0.0?
>>
>> Rohit, I think we HAVE to have concerns us on that question before
>> this merge happens.
>>
>> - chip
>>
>> Sent from my iPhone.
>>
>> On Dec 22, 2012, at 4:38 AM, Rohit Yadav <rohit.yadav@citrix.com> wrote:
>>
>>> Hi everyone,
>>>
>>> I'm planning to merge api_refactoring branch on to master after 72 hour
>> period which would be Monday EOD. Pl. go through the email, and previous
>> threads on api refactoring rework and feel free to share your ideas,
>> comments and vote to agree, disagree. If no one objects I would like to =
ask
>> the git Santa to merge it on Christmas 25 Dec :D (after 72 hour window)
>>>
>>> The reason why I want to merge around the next week is because I think
>> we would have lower frequencies of emails, review requests and people
>> contributing, hence I can move around a lot of code (mostly package
>> renames to org.apache.cloudstack in cloud-api) and right now the merge
>> conflicts are really minimum, about 100-200 lines. (A top level issues t=
o track
>> sub-issues: https://issues.apache.org/jira/browse/CLOUDSTACK-638)
>>>
>>> What will be affected:
>>> 0. Any class in cloud-api and on api-layer only
>>> 1. Any class that imports from/to cloud-api's response and cmd classes
>>>
>>> Some of the major changes that will be merged on master;
>>> 0. Over the wire (OTW) HTTP request to API server would send only UUID
>> strings. All requests done via UUIDs (and not CloudStack's internal db's=
 IDs).
>>> 1. https://issues.apache.org/jira/browse/CLOUDSTACK-518 Fix
>> @Parameter annotation to have annotation field to a Response class which
>> would give us entities (interface to VO objects). This would get rid of =
all
>> IdentityMapper using which was used earlier to get VO entities from an
>> annotated table name. This helps us to translate OTW UUIDs to CloudStack=
's
>> DB's internal IDs.
>>> 2. Separation of ACL Role access checker as an adapter, so organization=
s can
>> implement their own role based access checking. The mechanism would exis=
t
>> in CloudStack's API server but policy checking is moved out of CloudStac=
k.
>> (https://issues.apache.org/jira/browse/CLOUDSTACK-639) This works, but
>> was tough to get it right the first time, there is better way which I'll=
 share
>> before the merge.
>>> 3. Group APIs to
>> org.apache.cloudstack.api.{command,response}.{entity1,entity2 etc.}
>> packages. This is mainly done for developers, so when they work on API l=
ayer
>> they would know which api has what level of security and as they are
>> grouped based on entity type, it will be easier to search. This was most=
ly file
>> movement to org.apache.cloudstack.api package and helped us track couple
>> of classes which are no longer needed. Another aim was to move from
>> com.cloud to org.apache.cloudstack (only cloud-api for now).
>>> 4. Annotation work as described in 1., also for @ACL etc.
>>> 5. DB, ACL validation wip code
>>> 6. A lot of list api optimizations and response view work from our newe=
st
>> commiter, Min Chen. The aim is to simply response, right now for. exampl=
e
>> when we listVMs we don't want unnecessary (serialized) response objects
>> which could be queried using uuids separately.
>>>
>>> Pl. ask away any doubts, questions and concerns you may have. It was
>> challenging for me as well to understand the functional spec, to know th=
e
>> why/what/how, and if you read the old threads you can tell I did not get=
 it
>> the first time.
>>>
>>> A lot of annotation work is aimed to be completed over this weekend, so
>> when the branch is finally merged it won't break any functionality. At p=
resent
>> the branch is quite stable
>>>
>>> Testing and how or why do it?
>>> 0. Prasanna, Meghna? can help us write few basic sets of unit tests and
>> marvin integration tests for OTW requests. We already have few of their
>> patches on rb.
>>> 1. We can also have drivers to automate tests (Prasanna can talk more o=
n
>> this and on his devcloud based continuous intergration server)
>>> 2. If I do it now, there would be a lot more eyes to point out bugs and=
 I
>> want more people to participate in the refactoring work.
>>> 3. Right now, it builds and runs fine with minimal breaks and no
>> functionality breakage as most of the changes are only restricted to api=
-
>> server (:cloud-api artifact). I'm able to deploy a zone etc. To make the=
 UUID
>> thing work, I've put in hardcoded (for. ex. projectId=3D-1 which should =
be a
>> string uuid not a long int value -1) stuff that saves the UI from being =
broken
>> which I'll remove after merging on master so UI engineers can help fix U=
I
>> issues.
>>>
>>> Lastly, I would like to thank Min for her amazing patches and optimizat=
ion
>> work, Prachi for her work on ACL, Fang, Prasanna, Likitha for their help=
 with
>> the refactoring work and for their contributions. Community for asking
>> questions, raising issues and thanks to Alex for his guidance, reviews a=
nd
>> kickass OOP concepts.
>>>
>>> Ref;
>>> http://www.slideshare.net/buildacloud/cloudstack-collaboration-
>> conference-12-refactoring-cloud-stack
>>> https://git-wip-us.apache.org/repos/asf?p=3Dincubator-
>> cloudstack.git;a=3Dshortlog;h=3Drefs/heads/api_refactoring
>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/CloudStack+API+
>> refactoring
>>>
>>> Regards.
>>> PS. will write a blog on it this weekend so folks can follow what's goi=
ng on :)
>>> PPS. maybe explain in a video
>