Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: cloudstack-dev@incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of Alex.Huang@citrix.com
 designates 66.165.176.89 as permitted sender)
From: Alex Huang <Alex.Huang@citrix.com>
To: "cloudstack-dev@incubator.apache.org"
	<cloudstack-dev@incubator.apache.org>
Date: Sat, 22 Dec 2012 11:51:31 -0800
Subject: RE: Merge request: Merging api_refactoring on master
Thread-Topic: Merge request: Merging api_refactoring on master
Thread-Index: Ac3gfJsUAi2YeipYTkSR7LRO236/4QAAByPQ
Message-ID: 
 <B1DF26ECC0458748AC97CECE2DA98D41012FA385E352@SJCPMAILBOX01.citrite.net>
References: 
 <0BCCCE152323764BB7FD6AE6D7A1D9060102894ECFFE@BANPMAILBOX01.citrite.net>
 <-2095990210157392433@unknownmsgid>
 <B1DF26ECC0458748AC97CECE2DA98D41012FA385E34E@SJCPMAILBOX01.citrite.net>
 <-5478711158240320315@unknownmsgid>
 <B1DF26ECC0458748AC97CECE2DA98D41012FA385E34F@SJCPMAILBOX01.citrite.net>
 <-9081188622686871153@unknownmsgid>
In-Reply-To: <-9081188622686871153@unknownmsgid>
Accept-Language: en-US
Content-Language: en-US
acceptlanguage: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

John,

It's not JAX-RS that's the problem.  It's the fact that CloudStack current =
API was written to fit what the UI needs so therefore it pulls in a lot of =
items that doesn't belong to REST-ful entities.  So just switching to using=
 JAX-RS would mean a big change in the current work when the current work i=
s not designed correctly and mostly throw away.

As I mentioned in a previous mail, most of this work is designed to be reus=
able by a switch to a completely REST-ful api.  We'll be using  JAX-RS then=
.  I did a review of JAX-RS 1.1 and 2.0 and felt that it's still somewhat l=
acking but I think it can be augmented by other annotations to make it work=
 better. =20

--Alex

> -----Original Message-----
> From: John Burwell [mailto:jburwell@basho.com]
> Sent: Saturday, December 22, 2012 11:43 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: Merge request: Merging api_refactoring on master
>=20
> Alex,
>=20
> Just out of curiosity, what type of backwards compatibility issues
> prevented the adoption of a standard mechanism such as JAX-RS?
>=20
> Thanks,
> -John
>=20
> On Dec 22, 2012, at 2:40 PM, Alex Huang <Alex.Huang@citrix.com> wrote:
>=20
> > John,
> >
> > This refactoring did not use JAX-RS precisely because of the concern th=
at
> we needed to keep backwards compatibility.
> >
> > --Alex
> >
> >> -----Original Message-----
> >> From: John Burwell [mailto:jburwell@basho.com]
> >> Sent: Saturday, December 22, 2012 10:58 AM
> >> To: cloudstack-dev@incubator.apache.org
> >> Subject: Re: Merge request: Merging api_refactoring on master
> >>
> >> Alex,
> >>
> >> Is the refactoring based on JAX-RS or does it still use the custom
> >> REST mechanism?
> >>
> >> Thanks,
> >> -John
> >>
> >> 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
> consists
> >> of the following.
> >>>
> >>> - Moving java packages around for better grouping.  These doesn't hav=
e
> >> much impact on the query API, except for maybe some typos in the
> >> commands.properties file.
> >>> - Splitting the commands that have optional admin commands into an
> >> admin package.  The current commands.properties should still be
> referencing
> >> the admin 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
> >> returned 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 ra=
ther
> >> horrific fashion, causing problems in upgrade, performance, scalabilit=
y,
> and
> >> security.  We're really just cleaning up in terms of that.  If you're =
running
> 3.0-
> >> 4.0, you should be seeing uuids in the responses and using uuids in th=
e
> >> incoming query parameters already.  If you see specific examples where=
 it
> is
> >> not, it's a bug in the api.
> >>>
> >>> I don't think it will break the end user api other than bugs introduc=
ed
> during
> >> coding.  In fact, we took great effort to keep the api the same.  If w=
e
> didn't
> >> have that constraint, I would  have designed a completely new REST sty=
le
> 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 w=
hile
> >> moving a lot of the hard-coded parameter checking, security checking, =
etc
> >> into adapters to decouple the different aspects of checking to see 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 li=
st
> >>>> 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 wind=
ow)
> >>>>>
> >>>>> 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 issu=
es to
> >> 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 clas=
ses
> >>>>>
> >>>>> 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
> organizations
> >> can
> >>>> implement their own role based access checking. The mechanism
> would
> >> exist
> >>>> in CloudStack's API server but policy checking is moved out of
> CloudStack.
> >>>> (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
> >> layer
> >>>> 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
> mostly
> >> 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
> newest
> >>>> commiter, Min Chen. The aim is to simply response, right now for.
> >> example
> >>>> 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 wa=
s
> >>>> challenging for me as well to understand the functional spec, to kno=
w
> the
> >>>> 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
> >> present
> >>>> 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 mo=
re
> >> on
> >>>> 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 sho=
uld be
> a
> >>>> string uuid not a long int value -1) stuff that saves the UI from be=
ing
> >> broken
> >>>> which I'll remove after merging on master so UI engineers can help f=
ix
> UI
> >>>> issues.
> >>>>>
> >>>>> Lastly, I would like to thank Min for her amazing patches and
> >> optimization
> >>>> work, Prachi for her work on ACL, Fang, Prasanna, Likitha for their =
help
> >> with
> >>>> the refactoring work and for their contributions. Community for aski=
ng
> >>>> questions, raising issues and thanks to Alex for his guidance, revie=
ws
> and
> >>>> 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=
 going
> >> on :)
> >>>>> PPS. maybe explain in a video