> On Dec. 20, 2013, 6:11 a.m., Chris Mattmann wrote: > >
Please see comments below. Agree with the points not commented on > On Dec. 20, 2013, 6:11 a.m., Chris Mattmann wrote: > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/repository/PackagedWorkflowManager.java, > > line 2 > > <https://reviews.apache.org/r/15937/diff/1/?file=392691#file392691line2> > > > > I don't get what the point of this class is -- it seems to replicate > > functionality from the PackagedWorkflowRepository -- can you elaborate? It adds mainly the "saveWorkflow" functionality (i.e. persisting workflows). The other function (parsePackagedWorkflow) is just a wrapper around PackagedWorkflowRepository. Initially, I was thinking of adding this functionality in PackagedWorkflowRepository itself, but decided against it as I didn't want to touch the existing workflow code too much. > On Dec. 20, 2013, 6:11 a.m., Chris Mattmann wrote: > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/resources/WorkflowResource.java, > > line 89 > > <https://reviews.apache.org/r/15937/diff/1/?file=392693#file392693line89> > > > > would be great to add this read only method to packaged workflow repo > > in WM. The private function above is used to get existing *and* prospective files to write to. The PackagedWorkflowRepository class is provided a List of Files in the constructor. It is *not* provided the directory in which the files exist, or in which future workflow files would exist. So, it wouldn't be possible to create a prospective file name without making some assumptions about the list of files, or deriving some implications from it > On Dec. 20, 2013, 6:11 a.m., Chris Mattmann wrote: > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/resources/WorkflowResource.java, > > line 63 > > <https://reviews.apache.org/r/15937/diff/1/?file=392693#file392693line63> > > > > this should call the actual PackagedWorkflowRepository (since it's a > > read only operation). If you look at the parsePackagedWorkflow function, it's a wrapper around PackagedWorkflowRepository. It writes the workflowXML to a temporary file, creates a PackagedWorkflowRepository with only that file, and that fetches the workflow). - Varun ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15937/#review30735 ----------------------------------------------------------- On Dec. 1, 2013, 7 p.m., Varun Ratnakar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15937/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2013, 7 p.m.) > > > Review request for oodt and Chris Mattmann. > > > Bugs: OODT-563 > https://issues.apache.org/jira/browse/OODT-563 > > > Repository: oodt > > > Description > ------- > > This patch addresses the second part of > https://issues.apache.org/jira/browse/OODT-563. It adds a new webapp under > webapps/wmservices which provides: > 1. A servlet with Jax-RS functionality to Add/Remove workflows (just packaged > repository workflows for now) > > > Diffs > ----- > > trunk/webapp/wmservices/pom.xml PRE-CREATION > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/client/WmServicesClient.java > PRE-CREATION > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/repository/PackagedWorkflowManager.java > PRE-CREATION > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/resources/Resource.java > PRE-CREATION > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/resources/WorkflowResource.java > PRE-CREATION > > trunk/webapp/wmservices/src/main/java/org/apache/oodt/cas/wmservices/servlets/WmServicesServlet.java > PRE-CREATION > trunk/webapp/wmservices/src/main/webapp/META-INF/context.xml PRE-CREATION > trunk/webapp/wmservices/src/main/webapp/WEB-INF/web.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/15937/diff/ > > > Testing > ------- > > > Thanks, > > Varun Ratnakar > >
