You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Satish Saley <sa...@gmail.com> on 2017/01/23 17:22:03 UTC
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/
-----------------------------------------------------------
(Updated Jan. 23, 2017, 9:22 a.m.)
Review request for oozie.
Changes
-------
I have added more EL functions in this patch. Now the patch contains following
- endOfWeeks EL function for mentioning frequency.
- endOfDays, endOfWeeks, endOfMonths EL functions for mentioning sync dataset
Bugs: OOZIE-2630
https://issues.apache.org/jira/browse/OOZIE-2630
Repository: oozie-git
Description
-------
Oozie Coordinator EL Functions to get first day of the week/month
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
core/src/main/resources/oozie-default.xml ad10386
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
Diff: https://reviews.apache.org/r/51029/diff/
Testing
-------
Tested locally
Thanks,
Satish Saley
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/
-----------------------------------------------------------
(Updated Jan. 26, 2017, 4:42 p.m.)
Review request for oozie.
Bugs: OOZIE-2630
https://issues.apache.org/jira/browse/OOZIE-2630
Repository: oozie-git
Description
-------
Oozie Coordinator EL Functions to get first day of the week/month
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
core/src/main/resources/oozie-default.xml ad10386
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
Diff: https://reviews.apache.org/r/51029/diff/
Testing
-------
Tested locally
Thanks,
Satish Saley
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review163188
-----------------------------------------------------------
Ship it!
Ship It!
- Andr�s Piros
On Jan. 26, 2017, 10:27 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2017, 10:27 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/
-----------------------------------------------------------
(Updated Jan. 26, 2017, 2:27 p.m.)
Review request for oozie.
Bugs: OOZIE-2630
https://issues.apache.org/jira/browse/OOZIE-2630
Repository: oozie-git
Description
-------
Oozie Coordinator EL Functions to get first day of the week/month
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
core/src/main/resources/oozie-default.xml ad10386
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
Diff: https://reviews.apache.org/r/51029/diff/
Testing
-------
Tested locally
Thanks,
Satish Saley
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/
-----------------------------------------------------------
(Updated Jan. 26, 2017, 2:19 p.m.)
Review request for oozie.
Bugs: OOZIE-2630
https://issues.apache.org/jira/browse/OOZIE-2630
Repository: oozie-git
Description
-------
Oozie Coordinator EL Functions to get first day of the week/month
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
core/src/main/resources/oozie-default.xml ad10386
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
Diff: https://reviews.apache.org/r/51029/diff/
Testing
-------
Tested locally
Thanks,
Satish Saley
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review163166
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java (line 853)
<https://reviews.apache.org/r/51029/#comment234598>
you can remove them.
- Purshotam Shah
On Jan. 26, 2017, 2:48 a.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2017, 2:48 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/
-----------------------------------------------------------
(Updated Jan. 25, 2017, 6:48 p.m.)
Review request for oozie.
Bugs: OOZIE-2630
https://issues.apache.org/jira/browse/OOZIE-2630
Repository: oozie-git
Description
-------
Oozie Coordinator EL Functions to get first day of the week/month
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
core/src/main/resources/oozie-default.xml ad10386
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
Diff: https://reviews.apache.org/r/51029/diff/
Testing
-------
Tested locally
Thanks,
Satish Saley
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On Jan. 25, 2017, 10:14 p.m., Purshotam Shah wrote:
> > docs/src/site/twiki/CoordinatorFunctionalSpec.twiki, line 2290
> > <https://reviews.apache.org/r/51029/diff/5/?file=1613885#file1613885line2290>
> >
> > You can point it to 4.4.2.2. The coord:endOfMonths(int n) EL function
>
> Satish Saley wrote:
> The coord:endOfMonths(int n) EL function in 4.4.2.2 is for mentioning the freqency of the coordinator.
> Here we are documenting the EL function for dataset - 6.6.9. coord:endOfMonths(int n) EL Function for Synchronous Datasets
There is there is already an example of coord:endOfMonths(int n) in 4.4.2.2. For a user, it doesn't matter where they use coord:endOfMonths. It has the same meaning. For most of the part, we should point to 4.4.2.2.
- Purshotam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review163016
-----------------------------------------------------------
On Jan. 26, 2017, 10:27 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2017, 10:27 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
> On Jan. 25, 2017, 2:14 p.m., Purshotam Shah wrote:
> > core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java, line 749
> > <https://reviews.apache.org/r/51029/diff/5/?file=1613874#file1613874line749>
> >
> > This for coord timeout. We always consider 30 days for a month. If a job is submitted in Feb, then it will have 28 days for as timeout, and if the same job is submitted in Jan, it will have 31 days as timeout, which will be confusing.
I thought the same earlier, but later after looking at Rohini's comment https://reviews.apache.org/r/15893/#comment57211 I made the change. I will revert it back.
> On Jan. 25, 2017, 2:14 p.m., Purshotam Shah wrote:
> > docs/src/site/twiki/CoordinatorFunctionalSpec.twiki, line 408
> > <https://reviews.apache.org/r/51029/diff/5/?file=1613885#file1613885line408>
> >
> > Should be --+++++ 4.4.2.3. The coord:endOfWeeks(int n) EL function
It should be separate point 4.4.3.
4.4.2 is for month functions - The coord:months(int n) and coord:endOfMonths(int n) EL functions
> On Jan. 25, 2017, 2:14 p.m., Purshotam Shah wrote:
> > docs/src/site/twiki/CoordinatorFunctionalSpec.twiki, line 2290
> > <https://reviews.apache.org/r/51029/diff/5/?file=1613885#file1613885line2290>
> >
> > You can point it to 4.4.2.2. The coord:endOfMonths(int n) EL function
The coord:endOfMonths(int n) EL function in 4.4.2.2 is for mentioning the freqency of the coordinator.
Here we are documenting the EL function for dataset - 6.6.9. coord:endOfMonths(int n) EL Function for Synchronous Datasets
- Satish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review163016
-----------------------------------------------------------
On Jan. 24, 2017, 2:49 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review163016
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java (line 287)
<https://reviews.apache.org/r/51029/#comment234388>
can we just change coord:endOfMonthsRange to coord:absoluteRange. In that case, we can avoid extra function and configuration.
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java (line 300)
<https://reviews.apache.org/r/51029/#comment234389>
Can move them to different function like resolveInstanceRangeEndOfWeek.....
resolveInstanceRange function is getting longer and longer. You should be able to reuse a lot of code.
core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java (line 749)
<https://reviews.apache.org/r/51029/#comment234390>
This for coord timeout. We always consider 30 days for a month. If a job is submitted in Feb, then it will have 28 days for as timeout, and if the same job is submitted in Jan, it will have 31 days as timeout, which will be confusing.
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java (line 750)
<https://reviews.apache.org/r/51029/#comment234391>
Can we also have test case for dataset frequency coord:endOfWeeks. Most of the people will use coord:endOfWeeks for datasets.
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki (line 408)
<https://reviews.apache.org/r/51029/#comment234392>
Should be --+++++ 4.4.2.3. The coord:endOfWeeks(int n) EL function
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki (line 421)
<https://reviews.apache.org/r/51029/#comment234393>
Similar to endofMonths and endOfDays, coord:endOfWeeks example should have dataset frequency = coord:endOfWeeks.
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki (line 473)
<https://reviews.apache.org/r/51029/#comment234394>
---++++ 4.4.3. Cron syntax in coordinator frequency
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki (line 2290)
<https://reviews.apache.org/r/51029/#comment234395>
You can point it to 4.4.2.2. The coord:endOfMonths(int n) EL function
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki (line 2304)
<https://reviews.apache.org/r/51029/#comment234397>
frequency="${coord:months(1)}" should be daily. You want to run daily and process incremental data starting for the month.
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki (line 2355)
<https://reviews.apache.org/r/51029/#comment234398>
point to el function doc
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki (line 2475)
<https://reviews.apache.org/r/51029/#comment234401>
frequency="${coord:days(1)}, hourly
- Purshotam Shah
On Jan. 24, 2017, 10:49 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 10:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 591-606
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line591>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 610-626
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line610>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 683-697
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line683>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 702-717
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line702>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 721-736
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line721>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 802-817
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line802>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 838-854
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line838>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 572-586
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line572>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
- Satish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review162866
-----------------------------------------------------------
On Jan. 24, 2017, 2:49 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 521
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line521>
> >
> > `testCoord_endOfMonthTillMiddleOfNext_differenceIsCalculatedCorrectly()` would be a better name.
Throughout Oozie code camel caseing is used for naming test case methods. I updated the the method names to elaborate more.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 553
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line553>
> >
> > `testCoord_middleOfMonth_tillEndOfNext_differenceIsCalculatedCorrectly()` would be a better name.
Throughout Oozie code camel caseing is used for naming test case methods. I updated the the method names to elaborate more.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 590
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line590>
> >
> > `testCoord_startIsAfterEnd_throw()` would be a better name.
Throughout Oozie code camel caseing is used for naming test case methods. I updated the the method names to elaborate more.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 609
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line609>
> >
> > `testCoord_latestEnd_notSupported()` would be a better name.
Throughout Oozie code camel caseing is used for naming test case methods. I updated the the method names to elaborate more.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 630
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line630>
> >
> > `testCoord_endOfWeeksTillCurrent_correct()` would be a better name.
Throughout Oozie code camel caseing is used for naming test case methods. I updated the the method names to elaborate more.
- Satish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review162866
-----------------------------------------------------------
On Jan. 24, 2017, 2:49 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by András Piros <an...@cloudera.com>.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 572-586
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line572>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 591-606
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line591>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 610-626
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line610>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 683-697
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line683>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 702-717
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line702>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 721-736
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line721>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 802-817
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line802>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
> On Jan. 25, 2017, 2:14 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 838-854
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line838>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
- Andr�s
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review162866
-----------------------------------------------------------
On Jan. 24, 2017, 10:49 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 10:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 572-586
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line572>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I tried it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 591-606
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line591>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I spent couple of hours on it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 610-626
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line610>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I spent couple of hours on it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 683-697
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line683>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I spent couple of hours on it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 702-717
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line702>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I spent couple of hours on it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 721-736
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line721>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I spent couple of hours on it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 802-817
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line802>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> The exception that we need to check is a **wrapped exception**. We can check the exception with expected.expectCause from junit 4.11; but we cannot check the exception message string. Dropping this.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I spent couple of hours on it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
> On Jan. 24, 2017, 6:14 p.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 838-854
> > <https://reviews.apache.org/r/51029/diff/3-4/?file=1612950#file1612950line838>
> >
> > For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
> >
> > ```
> > @Rule
> > public ExpectedException expected = ExpectedException.none();
> > ...
> > expected.expect(MyOoziException.class);
> > expected.expectMessage(myExceptionMessageString);
> > performAndThrow();
> > ```
>
> Satish Saley wrote:
> Here I can use ExpectedException since exception is not wrapped. But as i mentioned in my earlier comments. ExpectedException is going to be removed from junit sooner. So I would rather stick to the try-catch.
>
> Andr�s Piros wrote:
> You could just define a custom `OozieCauseMatcher extends TypeSafeMatcher<Throwable>` as per:
>
> https://www.javacodegeeks.com/2014/03/junit-expectedexception-rule-beyond-basics.html
>
> but at least refactor to a method that does the `try-catch-assertCauseMessage` stuff for you and reuse that multiple times.
Thank you for the reference. I spent couple of hours on it, but i am unable to get it working. I think as long as test is serving its purpose, it should be ok to go with try-catch block. Also, if we go with ExpectedException, we need to make changes on junit 5.
- Satish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review162866
-----------------------------------------------------------
On Jan. 24, 2017, 2:49 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review162866
-----------------------------------------------------------
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 520)
<https://reviews.apache.org/r/51029/#comment234229>
`testCoord_endOfMonthTillMiddleOfNext_differenceIsCalculatedCorrectly()` would be a better name.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 538)
<https://reviews.apache.org/r/51029/#comment234230>
`testCoord_middleOfMonth_tillEndOfNext_differenceIsCalculatedCorrectly()` would be a better name.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 546)
<https://reviews.apache.org/r/51029/#comment234231>
I like that :)
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 551 - 559)
<https://reviews.apache.org/r/51029/#comment234280>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 563)
<https://reviews.apache.org/r/51029/#comment234253>
`testCoord_startIsAfterEnd_throw()` would be a better name.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 564 - 573)
<https://reviews.apache.org/r/51029/#comment234281>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 576)
<https://reviews.apache.org/r/51029/#comment234254>
`testCoord_latestEnd_notSupported()` would be a better name.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 577 - 585)
<https://reviews.apache.org/r/51029/#comment234282>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 589)
<https://reviews.apache.org/r/51029/#comment234278>
`testCoord_endOfWeeksTillCurrent_correct()` would be a better name.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 624 - 632)
<https://reviews.apache.org/r/51029/#comment234283>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 637 - 646)
<https://reviews.apache.org/r/51029/#comment234279>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 650 - 658)
<https://reviews.apache.org/r/51029/#comment234284>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 715 - 723)
<https://reviews.apache.org/r/51029/#comment234285>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 737 - 746)
<https://reviews.apache.org/r/51029/#comment234286>
For all similar `try-catch` stories like this, beginning w/ JUnit 4.7:
```
@Rule
public ExpectedException expected = ExpectedException.none();
...
expected.expect(MyOoziException.class);
expected.expectMessage(myExceptionMessageString);
performAndThrow();
```
- Andr�s Piros
On Jan. 24, 2017, 10:49 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 10:49 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/
-----------------------------------------------------------
(Updated Jan. 24, 2017, 2:49 p.m.)
Review request for oozie.
Changes
-------
Some changes in documentation
Bugs: OOZIE-2630
https://issues.apache.org/jira/browse/OOZIE-2630
Repository: oozie-git
Description
-------
Oozie Coordinator EL Functions to get first day of the week/month
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
core/src/main/resources/oozie-default.xml ad10386
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
Diff: https://reviews.apache.org/r/51029/diff/
Testing
-------
Tested locally
Thanks,
Satish Saley
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/
-----------------------------------------------------------
(Updated Jan. 24, 2017, 2:28 p.m.)
Review request for oozie.
Bugs: OOZIE-2630
https://issues.apache.org/jira/browse/OOZIE-2630
Repository: oozie-git
Description
-------
Oozie Coordinator EL Functions to get first day of the week/month
Diffs (updated)
-----
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
core/src/main/resources/oozie-default.xml ad10386
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
Diff: https://reviews.apache.org/r/51029/diff/
Testing
-------
Tested locally
Thanks,
Satish Saley
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by Satish Saley <sa...@gmail.com>.
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 582-585
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line582>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 598-601
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line598>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 681-684
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line681>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 697-700
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line697>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 712
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line712>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 714-717
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line714>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 781
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line781>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 783-786
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line783>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 797
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line797>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 799-802
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line799>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 814
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line814>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 816-819
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line816>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java, line 746
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612951#file1612951line746>
> >
> > You perform too much testing in one single test method.
> >
> > I'd have as many methods as testing use cases with appropriate naming strategy like http://osherove.com/blog/2005/4/3/naming-standards-for-unit-tests.html so that for every unit test we exactly know what it's testing and what is the expected behavior without needing to read the code, just reading the `Exception` logs.
This is a single test. We check that 4 actions should get materialized and time of materialization is correct. Dropping this.
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, lines 615-619
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line615>
> >
> > Use JUnit's `ExpectedException` instead:
> >
> > https://github.com/junit-team/junit4/wiki/exception-testing
I spent some time finding out the way to check wrapped exception message with ExpectedException, but there is no way to check the wrapped exception message string. Also, in future, (junit 5), ExpectedException is going to be removed. So dropping this.http://junit.org/junit5/docs/current/api/
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 534
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line534>
> >
> > Extract `DateUtils.parseDateOozieTZ("2009-08-20T01:00Z")` to a well-named variable.
Dropping this. With re-restructing of test cases, it won't be necessary.
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 545
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line545>
> >
> > Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a well-named variable.
Dropping this. With re-restructing of test cases, it won't be necessary.
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 551
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line551>
> >
> > Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a well-named variable.
Dropping this. With re-restructing of test cases, it won't be necessary.
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 559
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line559>
> >
> > Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a well-named variable.
Dropping this. With re-restructing of test cases, it won't be necessary.
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 568
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line568>
> >
> > Extract `DateUtils.parseDateOozieTZ("2009-07-01T01:00Z")` to a well-named variable.
Dropping this. With re-restructing of test cases, it won't be necessary.
> On Jan. 23, 2017, 11:47 a.m., Andr�s Piros wrote:
> > core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java, line 577
> > <https://reviews.apache.org/r/51029/diff/3/?file=1612950#file1612950line577>
> >
> > Extract `DateUtils.parseDateOozieTZ()` to a well-named variable.
Dropping this. With re-restructing of test cases, it won't be necessary.
- Satish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review162674
-----------------------------------------------------------
On Jan. 23, 2017, 9:22 a.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2017, 9:22 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>
Re: Review Request 51029: OOZIE 2630 Oozie Coordinator EL Functions
to get first day of the week/month
Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51029/#review162674
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java (line 283)
<https://reviews.apache.org/r/51029/#comment233994>
Please use `DAY_OF_MONTH` instead and externalize `1` to `FIRST_WEEK_OF_MONTH` for better readability.
core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java (line 302)
<https://reviews.apache.org/r/51029/#comment233996>
Calendars like European ones do not necessarily have Monday as the last day of week - use `calendar.getFirstDayOfWeek()` instead.
core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java (line 747)
<https://reviews.apache.org/r/51029/#comment233998>
Are you sure each and every month consists of exactly 30 days? I think it's a bug - I know not in the lines of code you touched, but indeed, maybe worth fixing it.
core/src/main/java/org/apache/oozie/util/DateUtils.java (line 307)
<https://reviews.apache.org/r/51029/#comment233999>
Calendars like European ones do not necessarily have Monday as the last day of week - use `calendar.getFirstDayOfWeek()` instead.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 521)
<https://reviews.apache.org/r/51029/#comment234000>
You perform too much testing in one single test method.
I'd have as many methods as testing use cases with appropriate naming strategy like http://osherove.com/blog/2005/4/3/naming-standards-for-unit-tests.html so that for every unit test we exactly know what it's testing and what is the expected behavior without needing to read the code, just reading the `Exception` logs.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 532)
<https://reviews.apache.org/r/51029/#comment234001>
Extract `DateUtils.parseDateOozieTZ("2009-08-20T01:00Z")` to a well-named variable.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 543)
<https://reviews.apache.org/r/51029/#comment234003>
Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a well-named variable.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 549)
<https://reviews.apache.org/r/51029/#comment234004>
Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a well-named variable.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 557)
<https://reviews.apache.org/r/51029/#comment234005>
Extract `DateUtils.parseDateOozieTZ("2009-08-01T01:00Z")` to a well-named variable.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 566)
<https://reviews.apache.org/r/51029/#comment234006>
Extract `DateUtils.parseDateOozieTZ("2009-07-01T01:00Z")` to a well-named variable.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 575)
<https://reviews.apache.org/r/51029/#comment234007>
Extract `DateUtils.parseDateOozieTZ()` to a well-named variable.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 580 - 583)
<https://reviews.apache.org/r/51029/#comment234008>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 596 - 599)
<https://reviews.apache.org/r/51029/#comment234009>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 613 - 617)
<https://reviews.apache.org/r/51029/#comment234010>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 679 - 682)
<https://reviews.apache.org/r/51029/#comment234011>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 695 - 698)
<https://reviews.apache.org/r/51029/#comment234015>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 710)
<https://reviews.apache.org/r/51029/#comment234016>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 712 - 715)
<https://reviews.apache.org/r/51029/#comment234017>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 719)
<https://reviews.apache.org/r/51029/#comment234019>
You perform too much testing in one single test method.
I'd have as many methods as testing use cases with appropriate naming strategy like http://osherove.com/blog/2005/4/3/naming-standards-for-unit-tests.html so that for every unit test we exactly know what it's testing and what is the expected behavior without needing to read the code, just reading the `Exception` logs.
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 779)
<https://reviews.apache.org/r/51029/#comment234020>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 781 - 784)
<https://reviews.apache.org/r/51029/#comment234021>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 795)
<https://reviews.apache.org/r/51029/#comment234022>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 797 - 800)
<https://reviews.apache.org/r/51029/#comment234023>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (line 812)
<https://reviews.apache.org/r/51029/#comment234024>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java (lines 814 - 817)
<https://reviews.apache.org/r/51029/#comment234025>
Use JUnit's `ExpectedException` instead:
https://github.com/junit-team/junit4/wiki/exception-testing
core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java (line 746)
<https://reviews.apache.org/r/51029/#comment234018>
You perform too much testing in one single test method.
I'd have as many methods as testing use cases with appropriate naming strategy like http://osherove.com/blog/2005/4/3/naming-standards-for-unit-tests.html so that for every unit test we exactly know what it's testing and what is the expected behavior without needing to read the code, just reading the `Exception` logs.
- Andr�s Piros
On Jan. 23, 2017, 5:22 p.m., Satish Saley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51029/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2017, 5:22 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-2630
> https://issues.apache.org/jira/browse/OOZIE-2630
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> Oozie Coordinator EL Functions to get first day of the week/month
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 0af7edc
> core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java 969336d
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 925a7aa
> core/src/main/java/org/apache/oozie/coord/TimeUnit.java 5b37639
> core/src/main/java/org/apache/oozie/util/DateUtils.java 3caf0a2
> core/src/main/resources/oozie-default.xml ad10386
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e69
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca1
> core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java be60133
> core/src/test/resources/coord-dataset-endOfDays.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfMonths.xml PRE-CREATION
> core/src/test/resources/coord-dataset-endOfWeeks.xml PRE-CREATION
> docs/src/site/twiki/CoordinatorFunctionalSpec.twiki e3ac514
>
> Diff: https://reviews.apache.org/r/51029/diff/
>
>
> Testing
> -------
>
> Tested locally
>
>
> Thanks,
>
> Satish Saley
>
>