You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Virag Kothari <vi...@yahoo-inc.com> on 2013/01/04 21:51:55 UTC
Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/
-----------------------------------------------------------
Review request for oozie.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1156
This addresses bug OOZIE-1156.
https://issues.apache.org/jira/browse/OOZIE-1156
Diffs
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428686
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1428686
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1428686
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1428686
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428686
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1428686
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1428686
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1428686
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
Diff: https://reviews.apache.org/r/8835/diff/
Testing
-------
Thanks,
Virag Kothari
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Virag Kothari <vi...@yahoo-inc.com>.
> On Jan. 8, 2013, 6:58 p.m., Mona Chitnis wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java, line 123
> > <https://reviews.apache.org/r/8835/diff/3/?file=245463#file245463line123>
> >
> > null/empty check for availDepList before the JPA executor is invoked?
discussed offline..not required as OOZIE-1161 takes care of it
> On Jan. 8, 2013, 6:58 p.m., Mona Chitnis wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java, line 594
> > <https://reviews.apache.org/r/8835/diff/3/?file=245464#file245464line594>
> >
> > rename to multiple i.e. createPartitionWrapper*s* for clarity
will do before committing
> On Jan. 8, 2013, 6:58 p.m., Mona Chitnis wrote:
> > branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml, line 63
> > <https://reviews.apache.org/r/8835/diff/3/?file=245467#file245467line63>
> >
> > typo? I dont see LOCAL_A
will remove the entire block from here and from coord-job-for-neg.xml before commit
- Virag
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/#review15149
-----------------------------------------------------------
On Jan. 7, 2013, 9:30 p.m., Virag Kothari wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8835/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2013, 9:30 p.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> This addresses bug OOZIE-1156.
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8835/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Virag Kothari
>
>
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/#review15149
-----------------------------------------------------------
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
<https://reviews.apache.org/r/8835/#comment32734>
null/empty check for availDepList before the JPA executor is invoked?
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/8835/#comment32735>
rename to multiple i.e. createPartitionWrapper*s* for clarity
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml
<https://reviews.apache.org/r/8835/#comment32736>
remove extra spaces
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml
<https://reviews.apache.org/r/8835/#comment32737>
typo? I dont see LOCAL_A
- Mona Chitnis
On Jan. 7, 2013, 9:30 p.m., Virag Kothari wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8835/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2013, 9:30 p.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> This addresses bug OOZIE-1156.
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8835/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Virag Kothari
>
>
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Mona Chitnis <mo...@yahoo.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/#review15155
-----------------------------------------------------------
Ship it!
Offline discussion with Virag about the review points. +1 after plural change and testcase minor comments
- Mona Chitnis
On Jan. 7, 2013, 9:30 p.m., Virag Kothari wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8835/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2013, 9:30 p.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> This addresses bug OOZIE-1156.
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8835/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Virag Kothari
>
>
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/#review15129
-----------------------------------------------------------
Ship it!
Ship It!
- Rohini Palaniswamy
On Jan. 7, 2013, 9:30 p.m., Virag Kothari wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8835/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2013, 9:30 p.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> This addresses bug OOZIE-1156.
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1430019
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1430019
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8835/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Virag Kothari
>
>
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/
-----------------------------------------------------------
(Updated Jan. 7, 2013, 9:30 p.m.)
Review request for oozie.
Changes
-------
Missed reverting the push missing dependency change in last diff.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1156
This addresses bug OOZIE-1156.
https://issues.apache.org/jira/browse/OOZIE-1156
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1430019
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1430019
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1430019
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1430019
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1430019
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1430019
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1430019
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
Diff: https://reviews.apache.org/r/8835/diff/
Testing
-------
Thanks,
Virag Kothari
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/
-----------------------------------------------------------
(Updated Jan. 7, 2013, 8:54 p.m.)
Review request for oozie.
Changes
-------
Updated based on Rohini's comments.
Unnecessary dB updates will be done as separate JIRA. Will change description and summary of JIRA
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1156
This addresses bug OOZIE-1156.
https://issues.apache.org/jira/browse/OOZIE-1156
Diffs (updated)
-----
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1429996
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1429996
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1429996
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1429996
branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1429996
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1429996
branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1429996
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
Diff: https://reviews.apache.org/r/8835/diff/
Testing
-------
Thanks,
Virag Kothari
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Virag Kothari <vi...@yahoo-inc.com>.
> On Jan. 5, 2013, 1:08 a.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java, line 429
> > <https://reviews.apache.org/r/8835/diff/1/?file=244992#file244992line429>
> >
> > Can you rename it to dependencyMap or better create a inner class with getters. Its confusing when reading the code.
Renamed to dependency Map.. The function has around 80 lines of code and is not well written. I thought of restructuring it but it will take some effort to rewrite it and can cause regression. So I dont prefer to do this as part of this JIRA.
> On Jan. 5, 2013, 1:08 a.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java, lines 472-479
> > <https://reviews.apache.org/r/8835/diff/1/?file=244992#file244992line472>
> >
> > Can you check if this will cause NPE elsewhere?
dependencyMap is local variable and checked other places for NPE
- Virag
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/#review15085
-----------------------------------------------------------
On Jan. 4, 2013, 8:51 p.m., Virag Kothari wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8835/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2013, 8:51 p.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> This addresses bug OOZIE-1156.
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1428686
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1428686
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1428686
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8835/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Virag Kothari
>
>
Re: Review Request: OOZIE-1156: Make all the latest/future instances as pull
dependences and remove unnecessary db updates for missing dependencies
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8835/#review15085
-----------------------------------------------------------
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/8835/#comment32671>
This doesn't work as we discussed. We can fix this in a separate jira. WE need a new JPAexecutor.
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
<https://reviews.apache.org/r/8835/#comment32678>
Can you rename it to dependencyMap or better create a inner class with getters. Its confusing when reading the code.
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
<https://reviews.apache.org/r/8835/#comment32677>
Can you check if this will cause NPE elsewhere?
branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
<https://reviews.apache.org/r/8835/#comment32680>
To be removed and addressed in a different jira with a new JPAExecutor
- Rohini Palaniswamy
On Jan. 4, 2013, 8:51 p.m., Virag Kothari wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8835/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2013, 8:51 p.m.)
>
>
> Review request for oozie.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> This addresses bug OOZIE-1156.
> https://issues.apache.org/jira/browse/OOZIE-1156
>
>
> Diffs
> -----
>
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java 1428686
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java 1428686
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 1428686
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java 1428686
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml PRE-CREATION
> branches/hcat-intre/core/src/test/resources/coord-job-for-matd-neg-hcat.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/8835/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Virag Kothari
>
>