cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Huang <Alex.Hu...@citrix.com>
Subject RE: Merge request: Merging api_refactoring on master
Date Sat, 22 Dec 2012 19:43:55 GMT
The other part to is also if we want to introduce a pure REST-ful API.  Most of the work we
did here can be reused.  Remember majority of this work is to pull the code that's hard coded
into adapters that other can use.  So a JAX-RS based implementation will basically be reusing
these adapters.   So this is a good in term solution to the REST-ful API.

--Alex

> -----Original Message-----
> From: Alex Huang [mailto:Alex.Huang@citrix.com]
> Sent: Saturday, December 22, 2012 11:40 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: RE: Merge request: Merging api_refactoring on master
> 
> John,
> 
> This refactoring did not use JAX-RS precisely because of the concern that 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 have
> > 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 rather
> > horrific fashion, causing problems in upgrade, performance, scalability, 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 the
> > 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 introduced
> during
> > 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 REST 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 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 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
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 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
> 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 was
> > >> challenging for me as well to understand the functional spec, to know
> 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 more
> > 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=-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
> 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 asking
> > >> questions, raising issues and thanks to Alex for his guidance, reviews
> 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=incubator-
> > >> cloudstack.git;a=shortlog;h=refs/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

Mime
View raw message