----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4755/#review7779 -----------------------------------------------------------
Looks good, I've made some comments inline. In addition: * I don't see the different oozie-site.xml for testing for different DBs being updated, is it not required? After your feedback I'll take the patch for a spin doing upgrades and creation tests. BTW, have you done that? Thanks, nice work. http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/META-INF/persistence.xml <https://reviews.apache.org/r/4755/#comment17087> please fix the indentation of these 3 classes http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/META-INF/persistence.xml <https://reviews.apache.org/r/4755/#comment17088> indentation fix http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/META-INF/persistence.xml <https://reviews.apache.org/r/4755/#comment17089> indentation fix http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/META-INF/persistence.xml <https://reviews.apache.org/r/4755/#comment17090> indentation fix http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml <https://reviews.apache.org/r/4755/#comment17086> you need to leave a space as value, else the value will be NULL. Looking at the code it really does not matter. So, this is just a minor NIT http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/AG_Install.twiki <https://reviews.apache.org/r/4755/#comment17091> we should add here the oozie.db.schema.name property with the default value, right? http://svn.apache.org/repos/asf/incubator/oozie/trunk/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java <https://reviews.apache.org/r/4755/#comment17093> we should trim the value obtained from the conf.get() here http://svn.apache.org/repos/asf/incubator/oozie/trunk/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java <https://reviews.apache.org/r/4755/#comment17096> can we please leave the names of table and columns with the case defined in the bean as they where before? else we could run into issues when a DB is case sensitive for table/column names. http://svn.apache.org/repos/asf/incubator/oozie/trunk/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java <https://reviews.apache.org/r/4755/#comment17097> why is this being remove? OpenJPA doe not do this as part of an upgrade, that is why this is here http://svn.apache.org/repos/asf/incubator/oozie/trunk/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java <https://reviews.apache.org/r/4755/#comment17098> trim value of conf.get() - Alejandro On 2012-05-09 02:36:58, Han Xiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4755/ > ----------------------------------------------------------- > > (Updated 2012-05-09 02:36:58) > > > Review request for oozie. > > > Summary > ------- > > Oozie's JPA service doesn't support configure different schemas for oozie's > db. > Our company wants to use one db for multi oozie clusters, and then we want > each oozie cluster can use different schema of the db. Therefore JPAService > in oozie should support to configure the schema of the db, not just the > database. > The oozie.db.schema.name is use to configure oozie's db name, however, it is > a little confusing for it is not used to configure the actually schema of db. > > > This addresses bug OOZIE-814. > https://issues.apache.org/jira/browse/OOZIE-814 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java > 1335881 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/conf/oozie-site.xml > 1335881 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/SystemInfoBean.java > PRE-CREATION > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/java/org/apache/oozie/service/JPAService.java > 1335881 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/META-INF/persistence.xml > 1335881 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/core/src/main/resources/oozie-default.xml > 1335881 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/docs/src/site/twiki/AG_Install.twiki > 1335881 > > http://svn.apache.org/repos/asf/incubator/oozie/trunk/utils/dbutils/updatescripts/readme.txt > 1335881 > > Diff: https://reviews.apache.org/r/4755/diff > > > Testing > ------- > > * Deployed Oozie, added oozie.db.schema.name configs values, succeed to run > the examples on Derby and Postgres databases. > > > Thanks, > > Han > >
