You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Ryota Egashira <eg...@yahoo-inc.com> on 2013/03/30 10:02:04 UTC

Re: Review Request: OOZIE-1297 Add chgrp in FS action

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/
-----------------------------------------------------------

(Updated March 30, 2013, 9:02 a.m.)


Review request for oozie.


Summary (updated)
-----------------

OOZIE-1297 Add chgrp in FS action


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1297


This addresses bug OOZIE-1297.
    https://issues.apache.org/jira/browse/OOZIE-1297


Diffs
-----

  trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
  trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 

Diff: https://reviews.apache.org/r/10204/diff/


Testing
-------

locally tested


Thanks,

Ryota Egashira


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Ryota Egashira <eg...@yahoo-inc.com>.

> On April 12, 2013, 6:29 p.m., Robert Kanter wrote:
> > trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd, line 64
> > <https://reviews.apache.org/r/10204/diff/4/?file=280751#file280751line64>
> >
> >     Sorry to keep nitpicking :) but there's also some lines that have tabs instead of spaces (they are also marked in red on RB)

yes, i'm in middle of fixing now :) also last uploaded one has error around schemaservice.  will upload modified one once done.


- Ryota


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/#review19091
-----------------------------------------------------------


On April 12, 2013, 5:41 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10204/
> -----------------------------------------------------------
> 
> (Updated April 12, 2013, 5:41 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> This addresses bug OOZIE-1297.
>     https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> Diffs
> -----
> 
>   trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
>   trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
>   trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 
> 
> Diff: https://reviews.apache.org/r/10204/diff/
> 
> 
> Testing
> -------
> 
> locally tested
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/#review19091
-----------------------------------------------------------



trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd
<https://reviews.apache.org/r/10204/#comment39640>

    Sorry to keep nitpicking :) but there's also some lines that have tabs instead of spaces (they are also marked in red on RB)


- Robert Kanter


On April 12, 2013, 5:41 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10204/
> -----------------------------------------------------------
> 
> (Updated April 12, 2013, 5:41 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> This addresses bug OOZIE-1297.
>     https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> Diffs
> -----
> 
>   trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
>   trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
>   trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 
> 
> Diff: https://reviews.apache.org/r/10204/diff/
> 
> 
> Testing
> -------
> 
> locally tested
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Ryota Egashira <eg...@yahoo-inc.com>.

> On April 18, 2013, 10:22 p.m., Virag Kothari wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java, line 185
> > <https://reviews.apache.org/r/10204/diff/5/?file=280760#file280760line185>
> >
> >     As the parsing logic is very similar to chmod, can we move it to a common method?

i created common method to recursively traverse directories, and make chgrp/chmod use it


> On April 18, 2013, 10:22 p.m., Virag Kothari wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java, line 211
> > <https://reviews.apache.org/r/10204/diff/5/?file=280760#file280760line211>
> >
> >     the entire logic is very similar to chmod. Can we combine this in a single method?

same above


- Ryota


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/#review19413
-----------------------------------------------------------


On April 12, 2013, 6:51 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10204/
> -----------------------------------------------------------
> 
> (Updated April 12, 2013, 6:51 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> This addresses bug OOZIE-1297.
>     https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> Diffs
> -----
> 
>   trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1467380 
>   trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1467380 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1467380 
>   trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1467380 
>   trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1467380 
> 
> Diff: https://reviews.apache.org/r/10204/diff/
> 
> 
> Testing
> -------
> 
> locally tested
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/#review19413
-----------------------------------------------------------



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java
<https://reviews.apache.org/r/10204/#comment40145>

    As the parsing logic is very similar to chmod, can we move it to a common method?



trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java
<https://reviews.apache.org/r/10204/#comment40146>

    the entire logic is very similar to chmod. Can we combine this in a single method?


- Virag Kothari


On April 12, 2013, 6:51 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10204/
> -----------------------------------------------------------
> 
> (Updated April 12, 2013, 6:51 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> This addresses bug OOZIE-1297.
>     https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> Diffs
> -----
> 
>   trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1467380 
>   trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1467380 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1467380 
>   trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1467380 
>   trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1467380 
> 
> Diff: https://reviews.apache.org/r/10204/diff/
> 
> 
> Testing
> -------
> 
> locally tested
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/#review19910
-----------------------------------------------------------

Ship it!


Ship It!

- Virag Kothari


On April 20, 2013, 1:49 a.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10204/
> -----------------------------------------------------------
> 
> (Updated April 20, 2013, 1:49 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> This addresses bug OOZIE-1297.
>     https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> Diffs
> -----
> 
>   trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
>   trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
>   trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 
> 
> Diff: https://reviews.apache.org/r/10204/diff/
> 
> 
> Testing
> -------
> 
> locally tested
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/
-----------------------------------------------------------

(Updated April 20, 2013, 1:49 a.m.)


Review request for oozie.


Changes
-------

fixed Virag's comments. took out the common logic among chmod/chgrp and make it one method.
since I changed chmod, i did sort of regression test using single-node cluster for all combinations (dir-files yes/no,  <recursive/> yes/no), and all worked. 
test case passed.


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1297


This addresses bug OOZIE-1297.
    https://issues.apache.org/jira/browse/OOZIE-1297


Diffs (updated)
-----

  trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
  trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
  trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
  trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 

Diff: https://reviews.apache.org/r/10204/diff/


Testing
-------

locally tested


Thanks,

Ryota Egashira


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/
-----------------------------------------------------------

(Updated April 12, 2013, 6:51 p.m.)


Review request for oozie.


Changes
-------

tried to fix all formatting issues. hope it works out this time.


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1297


This addresses bug OOZIE-1297.
    https://issues.apache.org/jira/browse/OOZIE-1297


Diffs (updated)
-----

  trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
  trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1467380 
  trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1467380 
  trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1467380 
  trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1467380 
  trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1467380 

Diff: https://reviews.apache.org/r/10204/diff/


Testing
-------

locally tested


Thanks,

Ryota Egashira


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/
-----------------------------------------------------------

(Updated April 12, 2013, 5:41 p.m.)


Review request for oozie.


Changes
-------

thanks for pointing it out, Robert.
removed trailing space.


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1297


This addresses bug OOZIE-1297.
    https://issues.apache.org/jira/browse/OOZIE-1297


Diffs (updated)
-----

  trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
  trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
  trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
  trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 

Diff: https://reviews.apache.org/r/10204/diff/


Testing
-------

locally tested


Thanks,

Ryota Egashira


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/#review19081
-----------------------------------------------------------



trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd
<https://reviews.apache.org/r/10204/#comment39630>

    There's a number of trailing whitespace and tab characters in oozie-workflow-0.4.5.xsd; please fix them.  RB highlights them in red


- Robert Kanter


On April 12, 2013, 7:57 a.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10204/
> -----------------------------------------------------------
> 
> (Updated April 12, 2013, 7:57 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> This addresses bug OOZIE-1297.
>     https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> Diffs
> -----
> 
>   trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
>   trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
>   trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 
> 
> Diff: https://reviews.apache.org/r/10204/diff/
> 
> 
> Testing
> -------
> 
> locally tested
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/
-----------------------------------------------------------

(Updated April 12, 2013, 7:57 a.m.)


Review request for oozie.


Changes
-------

got input from Mohammad on versioning of oozie-workflow.xsd, since this is minor change in schema, bump up to 0.4.5 instead of 0.5


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1297


This addresses bug OOZIE-1297.
    https://issues.apache.org/jira/browse/OOZIE-1297


Diffs (updated)
-----

  trunk/client/src/main/resources/oozie-workflow-0.4.5.xsd PRE-CREATION 
  trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
  trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
  trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 

Diff: https://reviews.apache.org/r/10204/diff/


Testing
-------

locally tested


Thanks,

Ryota Egashira


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/
-----------------------------------------------------------

(Updated April 12, 2013, 7:14 a.m.)


Review request for oozie.


Changes
-------

-created oozie-workflow-0.5.xsd which chgrp definition added.
-fixed some logic flaw and changed test cases
-tested all combinations using single-node cluster


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1297


This addresses bug OOZIE-1297.
    https://issues.apache.org/jira/browse/OOZIE-1297


Diffs (updated)
-----

  trunk/client/src/main/resources/oozie-workflow-0.5.xsd PRE-CREATION 
  trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
  trunk/core/src/main/java/org/apache/oozie/service/SchemaService.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
  trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
  trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 

Diff: https://reviews.apache.org/r/10204/diff/


Testing
-------

locally tested


Thanks,

Ryota Egashira


Re: Review Request: OOZIE-1297 Add chgrp in FS action

Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10204/#review19026
-----------------------------------------------------------


As a new element 'chrgp' is added, wouldn't a new schema be required?

- Virag Kothari


On March 30, 2013, 9:02 a.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10204/
> -----------------------------------------------------------
> 
> (Updated March 30, 2013, 9:02 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> This addresses bug OOZIE-1297.
>     https://issues.apache.org/jira/browse/OOZIE-1297
> 
> 
> Diffs
> -----
> 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/FsActionExecutor.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFsActionExecutor.java 1462649 
>   trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java 1462649 
>   trunk/docs/src/site/twiki/WorkflowFunctionalSpec.twiki 1462649 
> 
> Diff: https://reviews.apache.org/r/10204/diff/
> 
> 
> Testing
> -------
> 
> locally tested
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>