Re: Review Request: Updates from branch cas-pge

2012-03-10 Thread brian Foster 

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

(Updated 2012-03-09 08:17:12.801020)


Review request for oodt, Chris Mattmann and Paul Ramirez.


Summary
---

This includes the changes from wengine-branch cas-pge... still needs to add 
backwards compatibility for MetKeys (i.e. PCS_* etc)


This addresses bug OODT-262.
https://issues.apache.org/jira/browse/OODT-262


Diffs
-

  trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java 1297147 
  
trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java
 PRE-CREATION 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileStagingInfo.java 
PRE-CREATION 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfig.java 1297147 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigBuilder.java 
1297147 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigMetKeys.java 
1297147 
  
trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java
 1297147 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java 
PRE-CREATION 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java 
1297147 
  
trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java
 PRE-CREATION 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileStager.java 
PRE-CREATION 
  trunk/pge/src/main/java/org/apache/oodt/cas/pge/writers/PcsMetFileWriter.java 
1297147 

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


Testing
---


Thanks,

brian

Re: Review Request: Updates from branch cas-pge

2012-03-10 Thread brian Foster 


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > Looks good most of my comments are a request for some unit tests especially 
> > where stuff has been refactored and there is new code. Some of the stuff 
> > wasn't a change as the code was just reformatted.

Hey paul thanks for the comments, also should have noted that i'm gonna use 
this as like the master patch... will make a visual for me so i can create 
smaller incremental patches (if possible... lol)... lots of bug fixes and 
improvements were added to wengine's cas-pge... also cas-pge needs unit 
tests... currently only has one test which does nothing! lol... also needs 
cas-cli integration as well, so xml validation actions and the like can be 
written and possibly actions like workflow emulation mode actions so cas-pge 
can be run stand-alone but act like it is part of a workflow... this will 
probably take me the next century to accomplish but i'm gonna start chopping 
away at it :)


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 
> > 146-149
> > 
> >
> > Why is this gone? I see the new one that runs all. Mark this with 
> > deprecated? It's protected so only internally used just wondered.

not gone, signature changed to:

   protected void runPropertyAdder(ConfigFilePropertyAdder propAdder, PgeConfig 
pgeConfig, PgeMetadata pgeMetadata) {


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 
> > 238-239
> > 
> >
> > Not really a change just a reformatting.

The class lacked formatting... updated it to 3 space formatting


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, line 
> > 282
> > 
> >
> > no change just reformatting

The class lacked formatting... updated it to 3 space formatting


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 
> > 464-465
> > 
> >
> > Is this one of the keys you were saying is still needed in the patch?
> >

yup


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 
> > 548-550
> > 
> >
> > Where has this functionality moved to?

Still gotta work this back in... wengine's TaskInstance would update a tasks 
metadata for you... PgeMetadata existed in wengine as ControlMetadata


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java,
> >  line 41
> > 
> >
> > This appears to be a refactoring make sure the unit tests written for 
> > the XMLFilePGEConfigBuilder cover the code here.

will do


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java,
> >  line 69
> > 
> >
> > Lots of changes needs unit tests

will do!


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java,
> >  line 3
> > 
> >
> > yay constants.

acknowledged 


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java, 
> > line 241
> > 
> >
> > Yay unused code removed.

acknowledged


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java,
> >  line 1
> > 
> >
> > Needs unit tests. Also make sure there is coverage into FileStager.

will do!


- brian


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


On 2012-03-06 08:20:36, brian Foster wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4196/
> ---
> 
> (Updated 2012-03-06 08:20:36)
> 
> 
> Review request for oodt, Chris Mattmann and Paul Ramirez.
> 
> 
> Summary
> ---
> 
> This includ

Re: Review Request: Updates from branch cas-pge

2012-03-10 Thread Chris Mattmann 

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


Hey Brian, one of the ideas I had on this was to maintain like back compat with 
the existing PCS_ keys and not *force* folks to have to upgrade to them (which 
would force them to update the configs). One idea I had for doing this was to 
maintain like 2 interfaces, one for v1-keys (the "_" delimited ones), and 
another for v2-keys (the "/" delimited ones), and then allow folks using 
CAS-PGE to switch between them with a config file option in the pge-config.xml. 
Thoughts? I can work this up if you are +1.

- Chris


On 2012-03-09 08:17:12, brian Foster wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4196/
> ---
> 
> (Updated 2012-03-09 08:17:12)
> 
> 
> Review request for oodt, Chris Mattmann and Paul Ramirez.
> 
> 
> Summary
> ---
> 
> This includes the changes from wengine-branch cas-pge... still needs to add 
> backwards compatibility for MetKeys (i.e. PCS_* etc)
> 
> 
> This addresses bug OODT-262.
> https://issues.apache.org/jira/browse/OODT-262
> 
> 
> Diffs
> -
> 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java
>  PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileStagingInfo.java 
> PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfig.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigBuilder.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigMetKeys.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java
>  1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java 
> PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java
>  PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileStager.java 
> PRE-CREATION 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/writers/PcsMetFileWriter.java 
> 1297147 
> 
> Diff: https://reviews.apache.org/r/4196/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> brian
> 
>

Re: Review Request: Updates from branch cas-pge

2012-03-06 Thread Paul Ramirez 

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


Looks good most of my comments are a request for some unit tests especially 
where stuff has been refactored and there is new code. Some of the stuff wasn't 
a change as the code was just reformatted.


trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java


Why is this gone? I see the new one that runs all. Mark this with 
deprecated? It's protected so only internally used just wondered.



trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java


Not really a change just a reformatting.



trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java


no change just reformatting



trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java


Is this one of the keys you were saying is still needed in the patch?




trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java


Where has this functionality moved to?



trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java


This appears to be a refactoring make sure the unit tests written for the 
XMLFilePGEConfigBuilder cover the code here.



trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java


Lots of changes needs unit tests



trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java


yay constants.



trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java


Yay unused code removed.



trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java


Needs unit tests. Also make sure there is coverage into FileStager. 


- Paul


On 2012-03-06 08:20:36, brian Foster wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4196/
> ---
> 
> (Updated 2012-03-06 08:20:36)
> 
> 
> Review request for oodt, Chris Mattmann and Paul Ramirez.
> 
> 
> Summary
> ---
> 
> This includes the changes from wengine-branch cas-pge... still needs to add 
> backwards compatibility for MetKeys (i.e. PCS_* etc)
> 
> 
> Diffs
> -
> 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java
>  PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileStagingInfo.java 
> PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfig.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigBuilder.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigMetKeys.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java
>  1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java 
> PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java
>  PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileStager.java 
> PRE-CREATION 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/writers/PcsMetFileWriter.java 
> 1297147 
> 
> Diff: https://reviews.apache.org/r/4196/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> brian
> 
>

Re: Review Request: Updates from branch cas-pge

2012-03-06 Thread Chris Mattmann 

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


Hey brian, you can associate this one with 
https://issues.apache.org/jira/browse/OODT-262

- Chris


On 2012-03-06 08:20:36, brian Foster wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4196/
> ---
> 
> (Updated 2012-03-06 08:20:36)
> 
> 
> Review request for oodt, Chris Mattmann and Paul Ramirez.
> 
> 
> Summary
> ---
> 
> This includes the changes from wengine-branch cas-pge... still needs to add 
> backwards compatibility for MetKeys (i.e. PCS_* etc)
> 
> 
> Diffs
> -
> 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java
>  PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileStagingInfo.java 
> PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfig.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigBuilder.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigMetKeys.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java
>  1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java 
> PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java 
> 1297147 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java
>  PRE-CREATION 
>   trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileStager.java 
> PRE-CREATION 
>   
> trunk/pge/src/main/java/org/apache/oodt/cas/pge/writers/PcsMetFileWriter.java 
> 1297147 
> 
> Diff: https://reviews.apache.org/r/4196/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> brian
> 
>