----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13612/#review25281 -----------------------------------------------------------
Initial comments. Haven't gone through all the queries yet. Hope the next patch will contain old JPA Executor classes deleted. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java <https://reviews.apache.org/r/13612/#comment49587> Do we need QueryExecutorException or can we just leave it at JPAExecutorException? trunk/core/src/main/java/org/apache/oozie/command/wf/ActionCheckXCommand.java <https://reviews.apache.org/r/13612/#comment49588> Import to be removed. trunk/core/src/main/java/org/apache/oozie/executor/newjpa/BundleActionQueryExecutor.java <https://reviews.apache.org/r/13612/#comment49590> Javadoc needs to be fixed trunk/core/src/main/java/org/apache/oozie/executor/newjpa/BundleActionQueryExecutor.java <https://reviews.apache.org/r/13612/#comment49591> You need to do this in initialValue of the ThreadLocal. Same for other classes trunk/core/src/main/java/org/apache/oozie/executor/newjpa/BundleActionQueryExecutor.java <https://reviews.apache.org/r/13612/#comment49592> <E> is not needed here. Same in other classes trunk/core/src/main/java/org/apache/oozie/executor/newjpa/BundleActionQueryExecutor.java <https://reviews.apache.org/r/13612/#comment49593> default: //throw exception trunk/core/src/main/java/org/apache/oozie/executor/newjpa/BundleActionQueryExecutor.java <https://reviews.apache.org/r/13612/#comment49594> Is typecast required? trunk/core/src/main/java/org/apache/oozie/executor/newjpa/CoordinatorActionQueryExecutor.java <https://reviews.apache.org/r/13612/#comment49595> Some have FOR and some do not. Can follow one standard and remove the FOR from the query names. trunk/core/src/main/java/org/apache/oozie/executor/newjpa/CoordinatorActionQueryExecutor.java <https://reviews.apache.org/r/13612/#comment49596> This is duplicate of FOR_INPUT_CHECK trunk/core/src/main/java/org/apache/oozie/executor/newjpa/NamedQuery.java <https://reviews.apache.org/r/13612/#comment49597> Seems to be an unused class trunk/core/src/main/java/org/apache/oozie/executor/newjpa/QueryExecutor.java <https://reviews.apache.org/r/13612/#comment49600> Please keep the classes in jpa package. Let's not call it newjpa trunk/core/src/main/java/org/apache/oozie/executor/newjpa/QueryExecutor.java <https://reviews.apache.org/r/13612/#comment49598> Constructor should be protected. Also move jpaservice here as protected static and initialize in constructor if null. Same can be done for the threadlocal hashmap. It should also be protected static. trunk/core/src/main/java/org/apache/oozie/executor/newjpa/QueryExecutor.java <https://reviews.apache.org/r/13612/#comment49599> We should just do JPAExecutorException instead of changing code in lot of places. trunk/core/src/main/java/org/apache/oozie/service/CoordMaterializeTriggerService.java <https://reviews.apache.org/r/13612/#comment49589> Import to be removed. trunk/core/src/main/java/org/apache/oozie/service/JPAService.java <https://reviews.apache.org/r/13612/#comment49601> You will have to take maxresults as a parameter here. We can probably do these methods once we get to select queries. - Rohini Palaniswamy On Aug. 16, 2013, 1:17 a.m., Ryota Egashira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13612/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2013, 1:17 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1499 > https://issues.apache.org/jira/browse/OOZIE-1499 > > > Repository: oozie > > > Description > ------- > > https://issues.apache.org/jira/browse/OOZIE-1499 > > this is *WIP*, still rough, just uploading for early review on design. > > > Diffs > ----- > > trunk/core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 1513182 > trunk/core/src/main/java/org/apache/oozie/WorkflowActionBean.java 1513182 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java > 1513182 > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java > 1513182 > > trunk/core/src/main/java/org/apache/oozie/command/wf/ActionCheckXCommand.java > 1513182 > > trunk/core/src/main/java/org/apache/oozie/executor/jpa/BulkUpdateInsertForCoordActionStartJPAExecutor.java > 1513182 > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/BundleActionQueryExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/BundleJobQueryExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/CoordinatorActionQueryExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/CoordinatorJobQueryExecutor.java > PRE-CREATION > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/NamedQuery.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/QueryExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/QueryExecutorException.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/WorkflowActionQueryExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/executor/newjpa/WorkflowJobQueryExecutor.java > PRE-CREATION > > trunk/core/src/main/java/org/apache/oozie/service/CoordMaterializeTriggerService.java > 1513182 > trunk/core/src/main/java/org/apache/oozie/service/JPAService.java 1513182 > trunk/core/src/main/java/org/apache/oozie/store/CoordinatorStore.java > 1513182 > trunk/core/src/main/java/org/apache/oozie/store/WorkflowStore.java 1513182 > > trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java > 1513182 > > Diff: https://reviews.apache.org/r/13612/diff/ > > > Testing > ------- > > > Thanks, > > Ryota Egashira > >
