----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38474/#review101982 -----------------------------------------------------------
As a general point, can you add some additional comments/javadoc? Especially for the large new areas of code and the Jexl stuff. client/src/main/resources/oozie-coordinator-0.5.xsd (line 106) <https://reviews.apache.org/r/38474/#comment159491> Trailing whitespace. Also more of these throughout the patch. client/src/main/resources/oozie-coordinator-0.5.xsd (line 110) <https://reviews.apache.org/r/38474/#comment159492> Is there any way to enforce a max depth on nested "and"s and "or"s via the schema? client/src/main/resources/oozie-coordinator-0.5.xsd (line 132) <https://reviews.apache.org/r/38474/#comment159514> xs:int client/src/main/resources/oozie-coordinator-0.5.xsd (line 137) <https://reviews.apache.org/r/38474/#comment159515> xs:int core/pom.xml (line 373) <https://reviews.apache.org/r/38474/#comment159493> Version should be defined in root pom core/src/main/java/org/apache/oozie/CoordinatorActionBean.java <https://reviews.apache.org/r/38474/#comment159494> I'm not sure what this does exactly, but is it okay to delete? core/src/main/java/org/apache/oozie/action/ActionExecutor.java <https://reviews.apache.org/r/38474/#comment159498> "fake" change. core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 69) <https://reviews.apache.org/r/38474/#comment159504> so we don't lose the stack trace: ```` throw new IOException(e.getMessage, e); ```` core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 75) <https://reviews.apache.org/r/38474/#comment159505> Is there anyway to reuse this without always creating a new one everywhere? (is it not thread safe?) core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 120) <https://reviews.apache.org/r/38474/#comment159506> Should this be the OozieJexlEngine? core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 137) <https://reviews.apache.org/r/38474/#comment159507> Should this be the OozieJexlEngine? core/src/main/java/org/apache/oozie/coord/dependency/CoordDependency.java (lines 48 - 50) <https://reviews.apache.org/r/38474/#comment159508> Most of the time, these are re-assigned in the constructors. I think we can simpley declare them here instead of making new objects. core/src/main/java/org/apache/oozie/coord/dependency/CoordDependency.java (line 290) <https://reviews.apache.org/r/38474/#comment159509> isDependencyMet() core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOne.java (line 111) <https://reviews.apache.org/r/38474/#comment159510> We should probably do something better here core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOne.java (line 180) <https://reviews.apache.org/r/38474/#comment159511> We should probably do something better here core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseThree.java (line 78) <https://reviews.apache.org/r/38474/#comment159512> We should probably do something better here core/src/main/java/org/apache/oozie/coord/dependency/InputLogicParser.java (line 114) <https://reviews.apache.org/r/38474/#comment159513> wait isn't used? We also need to check that it's a valid int > 0 core/src/main/java/org/apache/oozie/service/SchemaService.java (line 75) <https://reviews.apache.org/r/38474/#comment159516> This is no longer in the code; it's now in oozie-default under oozie.service.SchemaService.coord.schemas. You'll probably hit this when you rebase. - Robert Kanter On Sept. 18, 2015, 12:20 a.m., Purshotam Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38474/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2015, 12:20 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1976 > https://issues.apache.org/jira/browse/OOZIE-1976 > > > Repository: oozie-git > > > Description > ------- > > There are three components in this patch > > 1. User interface > A new tag is added to coordinator.xml > ex. > <input-check> > <or name="test"> > <and> > <data-in dataset="A"/>" > <data-in dataset="B"/> > </and> > <and> > <data-in dataset="C"/> > <data-in dataset="D"/> > </and> > > </or>; > <input-check> > > > input-check will have nested and/or/combine operation. It can have min and > wait at operator or at date-in. > If input-check tag is missing then it consider to be old approach where all > data dependency are needed. > > 2. Processing > input-check is converted into logical expression > (a&&B)||(c&&d) > We use jexl to parse the logical expression. > > There are three phase in parsing. > phase 1 : only resolved dataset are parsed ( only current). > phase 2 : once all current are resolved, then future/latest are parsed. > phase 3 : Doesn't do any filecheck, just return what is being parsed by > phase1 and phase2. Is used for EL functions > > > 3. Storage. > if inputcheck is enable, push_missing_dependencies and missing_dependencies > are serialized and stored in DB. > If then not then it's old approach, where they are stored in plan text. This > is backward compatible. > > > Diffs > ----- > > client/src/main/resources/oozie-coordinator-0.5.xsd > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/pom.xml ca40e2e22293a3df2841764ce725420857425139 > core/src/main/java/org/apache/oozie/CoordinatorActionBean.java > 188b70e2e76858228b4d42e5798952383719a93d > core/src/main/java/org/apache/oozie/action/ActionExecutor.java > ff836fbbbe95ca03aace1136abea9548306b2cf2 > > core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java > a975f6edd40ef674638d1c32c36d5234b78beb0f > > core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java > 4e1c5b3392358cb6b1e98e16e469310338f27fed > core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java > ca8175e8b8019a42e2780fba4f8fbd313577494c > > core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 548946f05beacbba032b1f411fdae17ca7dd1f44 > > core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java > cc346274e2f28fd3eb062e8d3550281486c23954 > core/src/main/java/org/apache/oozie/coord/CoordELEvaluator.java > 8a279c06a62bd1e6502104f0760d7a1a9792b4bd > core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java > 7f59186bb508536e5662bc3ed25c7ec68ba1e2a7 > core/src/main/java/org/apache/oozie/coord/SyncCoordAction.java > 44258eb5be40bf6769e32c8780117e8533d80d7e > > core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/coord/dependency/CoordDependency.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOne.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOnePush.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseThree.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseTwo.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerUtility.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordInputDependencies.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordInputInstance.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordPullDependency.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordPushDependency.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/CoordUnResolvedDependency.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/coord/dependency/InputLogicParser.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/coord/dependency/OozieJexlEngine.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/coord/dependency/OozieJexlInterpreter.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/coord/dependency/OozieJexlNode.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/dependency/DependencyChecker.java > a5507575a3403103133f85a78c873c9976419857 > core/src/main/java/org/apache/oozie/service/SchemaService.java > 32105857f51eea9b2e4fd4d9d8cb74900fcdbac8 > core/src/main/java/org/apache/oozie/util/Pair.java > 1bf45b41edf19ad0a3115a7bafc010daef11b5c1 > core/src/main/resources/oozie-default.xml > 19cae9d4e07ea1c3cd3b287d23cacb9b342b406e > > core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java > f79c9a06c8238aa17b7d331afcb3f137f0bb83f9 > > core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java > 59b8b48b564b04ddd6eb263f8766bf3a8919a429 > > core/src/test/java/org/apache/oozie/coord/dependency/TestCoordInputCheckPush.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/test/java/org/apache/oozie/coord/dependency/TestCoordinatorInputCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/test/java/org/apache/oozie/coord/dependency/TestInputLogicParser.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/resources/coord-inputcheck-combine.xml > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/resources/coord-inputcheck-hcat.xml > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/resources/coord-inputcheck-latest.xml > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/resources/coord-inputcheck-range-latest.xml > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/resources/coord-inputcheck-range.xml > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/resources/coord-inputcheck.xml > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > Diff: https://reviews.apache.org/r/38474/diff/ > > > Testing > ------- > > > Thanks, > > Purshotam Shah > >
