You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2020/08/19 08:14:32 UTC

[GitHub] [submarine] JohnTing opened a new pull request #382: SUBMARINE-558.Define Swagger API for pre-defined template submission

JohnTing opened a new pull request #382:
URL: https://github.com/apache/submarine/pull/382


   ### What is this PR for?
   Make basic rest api for submi template.
   
   Convert submitted template to experiment
   post
   http://localhost/V1/template/submit
   
   
   ### What type of PR is it?
   [Bug Fix | Improvement | Feature | Documentation | Hot Fix | Refactoring]
   
   ### Todos
   * [ ] - API
   * [ ] - test
   * [ ] - e2e test
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-558
   
   ### How should this be tested?
   * First time? Setup Travis CI as described on https://submarine.apache.org/contribution/contributions.html#continuous-integration
   * Strongly recommended: add automated unit tests for any new or changed behavior
   * Outline any manual steps to test the PR here.
   
   ### Screenshots (if appropriate)
   
   ### Questions:
   * Does the licenses files need update? Yes/No
   * Is there breaking changes for older versions? Yes/No
   * Does this needs documentation? Yes/No
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
xunliu commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437305






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu removed a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
xunliu removed a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437305






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing commented on a change in pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #382:
URL: https://github.com/apache/submarine/pull/382#discussion_r475718360



##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experimenttemplate/ExperimentTemplateManager.java
##########
@@ -276,19 +280,56 @@ private ExperimentTemplate getExperimentTemplateDetails(String name) throws Subm
     return tpl;
   }
 
-  private ExperimentTemplateSpec parameterMapping(String spec) {
 
+  /**
+   * Create ExperimentTemplate
+   * 
+   * @param SubmittedParam experimentTemplate spec
+   * @return Experiment experiment
+   * @throws SubmarineRuntimeException the service error
+   */
+  public Experiment submitExperimentTemplate(ExperimentTemplateSubmit SubmittedParam) 
+        throws SubmarineRuntimeException {
+
+    if (SubmittedParam == null) {
+      throw new SubmarineRuntimeException(Status.BAD_REQUEST.getStatusCode(), 
+            "Invalid ExperimentTemplateSubmit spec.");
+    }
+
+    ExperimentTemplate experimentTemplate = getExperimentTemplate(SubmittedParam.getName());
+    Map<String, String> sparam = SubmittedParam.getParams();
+
+    for (ExperimentTemplateParamSpec tpaam: experimentTemplate.getExperimentTemplateSpec().getParameters()) {

Review comment:
       Thanks for the @pingsutw  review
   
   This change needs to make the following json format 
   
   ```json
   "experimentTemplateSpec": {
       "name": "tf-mnist-test",
       "author": "author",
       "description": "This is a template to run tf-mnist\n",
       "parameters": [
           {
               "name": "training.learning_rate",
               "required": "true",
               "description": " mnist learning_rate ",
               "value": "0.01"
           },
           {
               "name": "training.batch_size",
               "required": "false",
               "description": "This is batch size of training",
               "value": "150"
           }
       ],
   }
   ```
   into this
   
   ```json
   "experimentTemplateSpec": {
       "name": "tf-mnist-test",
       "author": "author",
       "description": "This is a template to run tf-mnist\n",
       "experimentTemplateParamSpec": [
           {
               "name": "training.learning_rate",
               "required": "true",
               "description": " mnist learning_rate ",
               "value": "0.01"
           },
           {
               "name": "training.batch_size",
               "required": "false",
               "description": "This is batch size of training",
               "value": "150"
           }
       ],
   }
   ```
   
   I am not sure if it is appropriate.
   If you still want to change please tell me again.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing edited a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing edited a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683304750


   @jiwq 
   If I only need to change the code place and path, there is no problem.
   Do I need to extend the API as POST /api/v1/experiment/submit or POST /api/v1/experiment/?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683304750


   @jiwq 
   If I only need to change the code place and path, there is no problem.
   Do I need to extend the API as POST /api/v1/experiment/submit or other?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] wangdatan commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
wangdatan commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-681198095


   Thanks @JohnTing for the patch, I reviewed registration flow, and what does the API looks like. I actually like the new implementation more than the original design of https://github.com/apache/submarine/blob/master/docs/design/experiment-implementation.md. The parameterized implementation is very clean and very easy for users to register and understand. Great job! 
   
   I will leave the detailed code review works to others, at a high-level, the implementation looks good! 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437695






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683298998


   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   
   
   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   
   
   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   @jiwq, Thanks for your review
   
   My last PR (SUBMARINE-559)
   [API] Define Swagger API for pre-defined template registration/delete, etc.
   Support methods such as create new template or delete template
   # get template list
   /api/v1/template
   
   # get template
   /api/v1/template/{templateName}
   
   # post template
   /api/v1/template
   
   # patch template
   /api/v1/template/{templateName}
   
   # delete template
   /api/v1/template/{templateName}
   
   then this PR uses the registered template to submit as an experiment
   post /api/v1/template/submit
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu removed a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
xunliu removed a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437305


   @JohnTing , What is the status of this PR now? Has this PR been completed?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
xunliu commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437305


   @JohnTing , What is the status of this PR now? Has this PR been completed?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437695






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] pingsutw commented on a change in pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #382:
URL: https://github.com/apache/submarine/pull/382#discussion_r475276259



##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experimenttemplate/ExperimentTemplateManager.java
##########
@@ -276,19 +280,56 @@ private ExperimentTemplate getExperimentTemplateDetails(String name) throws Subm
     return tpl;
   }
 
-  private ExperimentTemplateSpec parameterMapping(String spec) {
 
+  /**
+   * Create ExperimentTemplate
+   * 
+   * @param SubmittedParam experimentTemplate spec
+   * @return Experiment experiment
+   * @throws SubmarineRuntimeException the service error
+   */
+  public Experiment submitExperimentTemplate(ExperimentTemplateSubmit SubmittedParam) 
+        throws SubmarineRuntimeException {
+
+    if (SubmittedParam == null) {
+      throw new SubmarineRuntimeException(Status.BAD_REQUEST.getStatusCode(), 
+            "Invalid ExperimentTemplateSubmit spec.");
+    }
+
+    ExperimentTemplate experimentTemplate = getExperimentTemplate(SubmittedParam.getName());
+    Map<String, String> sparam = SubmittedParam.getParams();

Review comment:
       ```suggestion
       Map<String, String> params = SubmittedParam.getParams();
   
   ```

##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
##########
@@ -146,4 +174,56 @@ protected void verifyCreateExperimentTemplateApiResult(ExperimentTemplate tpl)
     Assert.assertNotNull(tpl.getExperimentTemplateSpec().getName());
     Assert.assertNotNull(tpl.getExperimentTemplateSpec());
   }
+
+  @Test
+  public void submitExperimentTemplate() throws Exception {
+
+    String body = loadContent(TPL_FILE);
+    run(body, "application/json");
+
+    String url = TPL_PATH + "/" + RestConstants.EXPERIMENT_TEMPLATE_SUBMIT;
+    // submit

Review comment:
       ```suggestion
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experimenttemplate/ExperimentTemplateManager.java
##########
@@ -297,27 +338,27 @@ private ExperimentTemplateSpec parameterMapping(String spec) {
 
     while (matcher.find()) {
       final String key = matcher.group(1);
-      final String replacement = parmMap.get(key);
+      final String replacement = paramMap.get(key);
       if (replacement == null) {
         unmappedKeys.add(key);
       }
       else {
         matcher.appendReplacement(sb, replacement);
       }
-      parmMap.remove(key);
+      paramMap.remove(key);
     }
     matcher.appendTail(sb);
 
-    if (parmMap.size() > 0) {
+    if (paramMap.size() > 0) {
       throw new SubmarineRuntimeException(Status.BAD_REQUEST.getStatusCode(),
-            "Parameters contains unused key: " + parmMap.keySet());
+            "Parameters contains unused key: " + paramMap.keySet());
     }
 
     if (unmappedKeys.size() > 0) {
       throw new SubmarineRuntimeException(Status.BAD_REQUEST.getStatusCode(),
           "Template contains unmapped key: " + unmappedKeys);
     }  
-
+    ExperimentTemplateSpec tplSpec;

Review comment:
       ```suggestion
       ExperimentTemplateSpec experimentTemplateSpec;
   
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experimenttemplate/ExperimentTemplateManager.java
##########
@@ -276,19 +280,56 @@ private ExperimentTemplate getExperimentTemplateDetails(String name) throws Subm
     return tpl;
   }
 
-  private ExperimentTemplateSpec parameterMapping(String spec) {
 
+  /**
+   * Create ExperimentTemplate
+   * 
+   * @param SubmittedParam experimentTemplate spec
+   * @return Experiment experiment
+   * @throws SubmarineRuntimeException the service error
+   */
+  public Experiment submitExperimentTemplate(ExperimentTemplateSubmit SubmittedParam) 
+        throws SubmarineRuntimeException {
+
+    if (SubmittedParam == null) {
+      throw new SubmarineRuntimeException(Status.BAD_REQUEST.getStatusCode(), 
+            "Invalid ExperimentTemplateSubmit spec.");
+    }
+
+    ExperimentTemplate experimentTemplate = getExperimentTemplate(SubmittedParam.getName());
+    Map<String, String> sparam = SubmittedParam.getParams();
+
+    for (ExperimentTemplateParamSpec tpaam: experimentTemplate.getExperimentTemplateSpec().getParameters()) {

Review comment:
       ```suggestion
       for (ExperimentTemplateParamSpec paramSpec: experimentTemplate.getExperimentTemplateSpec().getParameters()) {
   
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experimenttemplate/ExperimentTemplateManager.java
##########
@@ -276,19 +280,56 @@ private ExperimentTemplate getExperimentTemplateDetails(String name) throws Subm
     return tpl;
   }
 
-  private ExperimentTemplateSpec parameterMapping(String spec) {
 
+  /**
+   * Create ExperimentTemplate
+   * 
+   * @param SubmittedParam experimentTemplate spec
+   * @return Experiment experiment
+   * @throws SubmarineRuntimeException the service error
+   */
+  public Experiment submitExperimentTemplate(ExperimentTemplateSubmit SubmittedParam) 
+        throws SubmarineRuntimeException {
+
+    if (SubmittedParam == null) {
+      throw new SubmarineRuntimeException(Status.BAD_REQUEST.getStatusCode(), 
+            "Invalid ExperimentTemplateSubmit spec.");
+    }
+
+    ExperimentTemplate experimentTemplate = getExperimentTemplate(SubmittedParam.getName());
+    Map<String, String> sparam = SubmittedParam.getParams();
+
+    for (ExperimentTemplateParamSpec tpaam: experimentTemplate.getExperimentTemplateSpec().getParameters()) {

Review comment:
       I think we should change function name `experimentTemplate.getExperimentTemplateSpec().getParameters()` to `experimentTemplate.getExperimentTemplateSpec().getExperimentTemplateParamSpec()`
   
   https://github.com/apache/submarine/blob/517083e79f0bbe723a585831faf74f45aeaee8ed/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/spec/ExperimentTemplateSpec.java#L63-L68




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] asfgit closed pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #382:
URL: https://github.com/apache/submarine/pull/382






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing edited a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing edited a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683298998


   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   @jiwq, Thanks for your review
   
   My last PR (SUBMARINE-559)
   [API] Define Swagger API for pre-defined template registration/delete, etc.
   Support methods such as create new template or delete template
   ### get template list
   /api/v1/template
   
   ### get template
   get /api/v1/template/{templateName}
   
   ### post template
   post /api/v1/template
   
   ### patch template
   patch /api/v1/template/{templateName}
   
   ### delete template
   delete /api/v1/template/{templateName}
   
   then this PR uses the registered template to submit as an experiment
   ### submit template as an experiment
   post /api/v1/template/submit
   
   The format of the registered template includes a complete experiment, but the template maker can change some parameters into variables for template users to adjust.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437695


   > @JohnTing , What is the status of this PR now? Has this PR been completed?
   
   Thanks for @xunliu comment, this pr has been completed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing edited a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing edited a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683298998


   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   
   
   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   
   
   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   @jiwq, Thanks for your review
   
   My last PR (SUBMARINE-559)
   [API] Define Swagger API for pre-defined template registration/delete, etc.
   Support methods such as create new template or delete template
   ### get template list
   /api/v1/template
   
   ### get template
   get /api/v1/template/{templateName}
   
   ### post template
   post /api/v1/template
   
   ### patch template
   patch /api/v1/template/{templateName}
   
   ### delete template
   delete /api/v1/template/{templateName}
   
   then this PR uses the registered template to submit as an experiment
   ### submit template as an experiment
   post /api/v1/template/submit
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] jiwq commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
jiwq commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683302819


   > > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   > 
   > @jiwq, Thanks for your review
   > 
   > My last PR (SUBMARINE-559)
   > [API] Define Swagger API for pre-defined template registration/delete, etc.
   > Support methods such as create new template or delete template
   > 
   > ### get template list
   > /api/v1/template
   > 
   > ### get template
   > get /api/v1/template/{templateName}
   > 
   > ### post template
   > post /api/v1/template
   > 
   > ### patch template
   > patch /api/v1/template/{templateName}
   > 
   > ### delete template
   > delete /api/v1/template/{templateName}
   > 
   > then this PR uses the registered template to submit as an experiment
   > 
   > ### submit template as an experiment
   > post /api/v1/template/submit
   > 
   > The format of the registered template includes a complete experiment, but the template maker can change some parameters into variables for template users to adjust.
   
   Got. I think `POST /api/v1/template/submit` is not a good design, extend the `POST /api/v1/experiment` is the better selected (more info see mine above comment). Any thoughts?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing edited a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing edited a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683298998


   > @JohnTing Thanks for your contribution. But I have a doubt "how can the user submit the experiment by template?" I saw the REST api `/api/v1/template/submit`, but I can't get its meaning. Such as 'create new template' or others? In my opinion, if we provide the template resource, the experiment resource depends on template. So we should extend the experiment api to support accepts the template id and related params to training.
   
   @jiwq, Thanks for your review
   
   My last PR (SUBMARINE-559)
   [API] Define Swagger API for pre-defined template registration/delete, etc.
   Support methods such as create new template or delete template
   ### get template list
   /api/v1/template
   
   ### get template
   get /api/v1/template/{templateName}
   
   ### post template
   post /api/v1/template
   
   ### patch template
   patch /api/v1/template/{templateName}
   
   ### delete template
   delete /api/v1/template/{templateName}
   
   then this PR uses the registered template to submit as an experiment
   ### submit template as an experiment
   post /api/v1/template/submit
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing commented on a change in pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #382:
URL: https://github.com/apache/submarine/pull/382#discussion_r475718360



##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experimenttemplate/ExperimentTemplateManager.java
##########
@@ -276,19 +280,56 @@ private ExperimentTemplate getExperimentTemplateDetails(String name) throws Subm
     return tpl;
   }
 
-  private ExperimentTemplateSpec parameterMapping(String spec) {
 
+  /**
+   * Create ExperimentTemplate
+   * 
+   * @param SubmittedParam experimentTemplate spec
+   * @return Experiment experiment
+   * @throws SubmarineRuntimeException the service error
+   */
+  public Experiment submitExperimentTemplate(ExperimentTemplateSubmit SubmittedParam) 
+        throws SubmarineRuntimeException {
+
+    if (SubmittedParam == null) {
+      throw new SubmarineRuntimeException(Status.BAD_REQUEST.getStatusCode(), 
+            "Invalid ExperimentTemplateSubmit spec.");
+    }
+
+    ExperimentTemplate experimentTemplate = getExperimentTemplate(SubmittedParam.getName());
+    Map<String, String> sparam = SubmittedParam.getParams();
+
+    for (ExperimentTemplateParamSpec tpaam: experimentTemplate.getExperimentTemplateSpec().getParameters()) {

Review comment:
       This change needs to make the following json format 
   
   ```json
   "experimentTemplateSpec": {
       "name": "tf-mnist-test",
       "author": "author",
       "description": "This is a template to run tf-mnist\n",
       "parameters": [
           {
               "name": "training.learning_rate",
               "required": "true",
               "description": " mnist learning_rate ",
               "value": "0.01"
           },
           {
               "name": "training.batch_size",
               "required": "false",
               "description": "This is batch size of training",
               "value": "150"
           }
       ],
   }
   ```
   into this
   
   ```json
   "experimentTemplateSpec": {
       "name": "tf-mnist-test",
       "author": "author",
       "description": "This is a template to run tf-mnist\n",
       "experimentTemplateParamSpec": [
           {
               "name": "training.learning_rate",
               "required": "true",
               "description": " mnist learning_rate ",
               "value": "0.01"
           },
           {
               "name": "training.batch_size",
               "required": "false",
               "description": "This is batch size of training",
               "value": "150"
           }
       ],
   }
   ```
   
   I am not sure if it is appropriate.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] JohnTing edited a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
JohnTing edited a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-683304750


   @jiwq 
   If I only need to change the code place and path, there is no problem.
   Do I need to extend the API as POST /api/v1/experiment/submit or other path?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu removed a comment on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
xunliu removed a comment on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437305






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] asfgit closed pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #382:
URL: https://github.com/apache/submarine/pull/382


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] jiwq commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
jiwq commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-687575380


   > @jiwq
   > If I only need to change the code place and path, there is no problem.
   > Do I need to extend the API as POST /api/v1/experiment/submit or POST /api/v1/experiment/?
   
   I recommend use `/api/v1/experiment` as the resource and extend the associated operations.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] asfgit closed pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #382:
URL: https://github.com/apache/submarine/pull/382






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
xunliu commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437305


   @JohnTing , What is the status of this PR now? Has this PR been completed?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [submarine] xunliu commented on pull request #382: SUBMARINE-558. Define Swagger API for pre-defined template submission

Posted by GitBox <gi...@apache.org>.
xunliu commented on pull request #382:
URL: https://github.com/apache/submarine/pull/382#issuecomment-691437305


   @JohnTing , What is the status of this PR now? Has this PR been completed?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org