-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6557/#review10225
-----------------------------------------------------------


This is the wrong way to do this.  I thought the agreement is to use the 
GenericDao stuff for now so that when we switch that layer we can switch 
everything together.

Here's also what Darren said on the mailing list:

I was just reviewing https://reviews.apache.org/r/6557/diff/ for the Hibernate 
-> Custom DAO implementation and it makes me so sad.  Its not that you went 
from Hibernate ORM to custom built ORM (that is already in CloudStack), you 
went from ORM to raw SQL.  From the size of the diff and the amount of lines of 
code added, its an advertisement in itself in why one would want to use an ORM.

I just joined this mail listing so I probably missed the whole back story, but 
from what I can gather, you can't use hibernate because of the license and the 
consensus is to just make it consistent with CloudStack, which already has a 
custom ORM/DAO implementation.

This comment is probably too late in the game, but I really think your missing 
an opportunity here.  For the future of CloudStack I think most everyone would 
agree that it should move to a standard ORM approach and drop the custom one it 
has.  Using my "Jump to Conclusions Mat" I would say that such a discussion 
will end up with deciding that Spring+JPA should be the interface/container and 
the implementation I don't know (I'd vote Eclipse TopLink if the license is 
compatible).  

So the AWS component is a perfect place to test out such framework do to its 
isolated nature.  Having a lot of experience with hibernate/jpa and CloudStack 
I have a good idea in my mind just how difficult it would be to convert all of 
the CloudStack code to a JPA compatible solution.  If you were to do the 
conversion, for practical reasons, you are going to have to do a phased 
migration.  This means that you'd have to support old framework and new 
framework concurrently for sometime.  Coming up with an approach to reconcile 
the two frameworks will take some time and effort.  Since the EC2 component is 
a pretty well isolated framework, it would be easy to implement Spring/JPA and 
test the validity of what would be the end state before you take on the complex 
and long task of converting all of CloudStack.

I'd even be willing to work on this.  It just seems silly to me to go from ORM 
to SQL and then back to ORM.  But I realize, as I said before, this comment is 
somewhat late in the game.

 

- Alex Huang


On Aug. 12, 2012, 9:31 a.m., Rajesh Battala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6557/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2012, 9:31 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Chiradeep Vittal.
> 
> 
> Description
> -------
> 
> S3 API service in CloudStack is using Hibernate ORM Framework. Hibernate is 
> licensed under the LGPL v2.1. 
> Hibernate dependency is removed by moving S3/EC2 to DAO Omplementation.
> 
> 
> This addresses bugs CS-15314 and CS-15686.
> 
> 
> Diffs
> -----
> 
>   awsapi/.classpath 4dc46c4 
>   awsapi/conf/CloudStack.cfg.xml 511e68f 
>   awsapi/conf/hibernate.cfg.xml d484849 
>   awsapi/src/com/cloud/bridge/lifecycle/ServiceEngineLifecycle.java 22adfb8 
>   awsapi/src/com/cloud/bridge/model/MHost.java 2832e1a 
>   awsapi/src/com/cloud/bridge/model/MHostMount.java d532568 
>   awsapi/src/com/cloud/bridge/model/SAcl.java bff7906 
>   awsapi/src/com/cloud/bridge/model/SBucket.java 5f63a77 
>   awsapi/src/com/cloud/bridge/model/SHost.java 874b095 
>   awsapi/src/com/cloud/bridge/model/SMeta.java 8962771 
>   awsapi/src/com/cloud/bridge/model/SObject.java 73a2c9a 
>   awsapi/src/com/cloud/bridge/model/SObjectItem.java 1429089 
>   awsapi/src/com/cloud/bridge/persist/EntityDao.java c7943a6 
>   awsapi/src/com/cloud/bridge/persist/GMTDateTimeUserType.java b39f298 
>   awsapi/src/com/cloud/bridge/persist/PersistContext.java 8abc5da 
>   awsapi/src/com/cloud/bridge/persist/PersistException.java e2acfd2 
>   awsapi/src/com/cloud/bridge/persist/dao/CloudStackAccountDao.java 31a5be8 
>   awsapi/src/com/cloud/bridge/persist/dao/CloudStackConfigurationDao.java 
> ed16974 
>   awsapi/src/com/cloud/bridge/persist/dao/CloudStackSvcOfferingDao.java 
> 5013eac 
>   awsapi/src/com/cloud/bridge/persist/dao/MHostDao.java 1c5e81c 
>   awsapi/src/com/cloud/bridge/persist/dao/MHostMountDao.java 6753e9c 
>   awsapi/src/com/cloud/bridge/persist/dao/SAclDao.java 37f18f3 
>   awsapi/src/com/cloud/bridge/persist/dao/SBucketDao.java 2584597 
>   awsapi/src/com/cloud/bridge/persist/dao/SHostDao.java 2641fdb 
>   awsapi/src/com/cloud/bridge/persist/dao/SMetaDao.java 1bc7933 
>   awsapi/src/com/cloud/bridge/persist/dao/SObjectDao.java ac00e03 
>   awsapi/src/com/cloud/bridge/persist/dao/SObjectItemDao.java 62b424e 
>   awsapi/src/com/cloud/bridge/persist/dao/UserCredentialsDao.java 4beb7a3 
>   awsapi/src/com/cloud/bridge/service/EC2MainServlet.java 0c904a1 
>   awsapi/src/com/cloud/bridge/service/EC2RestServlet.java ceaf59d 
>   awsapi/src/com/cloud/bridge/service/S3RestServlet.java 282385b 
>   awsapi/src/com/cloud/bridge/service/controller/s3/S3BucketAction.java 
> 39b5cf6 
>   awsapi/src/com/cloud/bridge/service/controller/s3/S3ObjectAction.java 
> 0aba4ba 
>   awsapi/src/com/cloud/bridge/service/controller/s3/ServiceProvider.java 
> 7a36a4b 
>   awsapi/src/com/cloud/bridge/service/core/ec2/EC2Engine.java 1363d0d 
>   awsapi/src/com/cloud/bridge/service/core/s3/S3Engine.java e8b73a4 
>   awsapi/src/com/cloud/bridge/util/CloudSessionFactory.java 404f463 
>   awsapi/src/com/cloud/bridge/util/CloudStackSessionFactory.java a5e97b8 
>   awsapi/src/com/cloud/bridge/util/QueryHelper.java 1a1b582 
>   awsapi/src/com/cloud/stack/models/CloudStackServiceOffering.java 3dcf011 
>   deps/awsapi-lib/hibernate3.jar 33755a45fa81dbb318961eccdb8a1bbd8419e61d 
> 
> Diff: https://reviews.apache.org/r/6557/diff/
> 
> 
> Testing
> -------
> 
> Testing done
> 1. Bucket Operations
> 2. Key Operations
> with boto client.
> 
> 
> Thanks,
> 
> Rajesh Battala
> 
>

Reply via email to