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/03 08:54:48 UTC

[GitHub] [submarine] pingsutw opened a new pull request #369: SUBMARINE-590. [WEB] Update chart/param/metric UI

pingsutw opened a new pull request #369:
URL: https://github.com/apache/submarine/pull/369


   ### What is this PR for?
   - Improve chart/param/metric UI
   - Fix the database connection error due to the update in SUBMARINE-575
   - Read the data from the `metric` table, and draw charts in workbench
   
   Following commands could reproduce the experiment
   ```bash
   curl -X POST -H "Content-Type: application/json" -d '
   {
     "meta": {
       "name": "tf-mnist2",
       "namespace": "submarine",
       "framework": "TensorFlow",
       "cmd": "python3 /var/example/tracking.py",
       "envVars": {
         "ENV_1": "ENV1"
       }
     },
     "environment": {
       "image": "pingsutw/tracking-example:0.5.0.dev"
     },
     "spec": {
       "Ps": {
         "replicas": 1,
         "resources": "cpu=1,memory=1024M"
       },
       "Worker": {
         "replicas": 1,
         "resources": "cpu=1,memory=1024M"
       }
     }
   }
   ' http://127.0.0.1:8080/api/v1/experiment
   
   ```
   
   ### What type of PR is it?
   [Improvement]
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/browse/SUBMARINE-590
   
   ### How should this be tested?
   https://travis-ci.org/github/pingsutw/hadoop-submarine/builds/714396434
   
   ### Screenshots (if appropriate)
   ![Screenshot from 2020-08-03 12-42-34](https://user-images.githubusercontent.com/37936015/89163413-1f071000-d5a8-11ea-9b44-c7646ccb2868.png)
   
   ![Screenshot from 2020-08-03 12-42-35](https://user-images.githubusercontent.com/37936015/89163445-29290e80-d5a8-11ea-943e-e59eb3c753f4.png)
   
   ![Screenshot from 2020-08-03 12-42-38](https://user-images.githubusercontent.com/37936015/89163458-2d552c00-d5a8-11ea-9c3a-c250f06f32ef.png)
   
   ![Screenshot from 2020-08-03 12-42-37](https://user-images.githubusercontent.com/37936015/89163454-2af2d200-d5a8-11ea-9274-52fc5ccdb7a5.png)
   
   ### Questions:
   * Does the licenses files need update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? 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] jiwq commented on a change in pull request #369: SUBMARINE-590. [WEB] Update chart/param/metric UI

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



##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
##########
@@ -85,13 +87,33 @@ private ExperimentManager(Submitter submitter) {
    */
   public Experiment createExperiment(ExperimentSpec spec) throws SubmarineRuntimeException {
     checkSpec(spec);
+
+    // Submarine sdk will get experimentID and JDBC URL from environment variables in each worker,
+    // and then log experiment metrics and parameters to submarine server
+    ExperimentId id = generateExperimentId();
+    String url = getSQLalchemyURL();

Review comment:
       ```suggestion
       String url = getSQLAlchemyURL();
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
##########
@@ -85,13 +87,33 @@ private ExperimentManager(Submitter submitter) {
    */
   public Experiment createExperiment(ExperimentSpec spec) throws SubmarineRuntimeException {
     checkSpec(spec);
+
+    // Submarine sdk will get experimentID and JDBC URL from environment variables in each worker,
+    // and then log experiment metrics and parameters to submarine server
+    ExperimentId id = generateExperimentId();
+    String url = getSQLalchemyURL();
+    spec.getMeta().getEnvVars().put(RestConstants.JOB_ID, id.toString());
+    spec.getMeta().getEnvVars().put(RestConstants.SUBMARINE_TRACKING_URI, url);
+
     Experiment experiment = submitter.createExperiment(spec);
-    experiment.setExperimentId(generateExperimentId());
+    experiment.setExperimentId(id);
+
+    spec.getMeta().getEnvVars().remove(RestConstants.JOB_ID);
+    spec.getMeta().getEnvVars().remove(RestConstants.SUBMARINE_TRACKING_URI);
     experiment.setSpec(spec);
     cachedExperimentMap.putIfAbsent(experiment.getExperimentId().toString(), experiment);
     return experiment;
   }
 
+  private String getSQLalchemyURL() {

Review comment:
       ```suggestion
     private String getSQLAlchemyURL() {
   ```




----------------------------------------------------------------
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] tangzhankun commented on a change in pull request #369: SUBMARINE-590. [WEB] Update chart/param/metric UI

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



##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
##########
@@ -85,13 +87,33 @@ private ExperimentManager(Submitter submitter) {
    */
   public Experiment createExperiment(ExperimentSpec spec) throws SubmarineRuntimeException {
     checkSpec(spec);
+
+    // Submarine sdk will get experimentID and JDBC URL from environment variables in each worker,
+    // and then log experiment metrics and parameters to submarine server
+    ExperimentId id = generateExperimentId();
+    String url = getSQLAlchemyURL();
+    spec.getMeta().getEnvVars().put(RestConstants.JOB_ID, id.toString());
+    spec.getMeta().getEnvVars().put(RestConstants.SUBMARINE_TRACKING_URI, url);
+
     Experiment experiment = submitter.createExperiment(spec);
-    experiment.setExperimentId(generateExperimentId());
+    experiment.setExperimentId(id);
+
+    spec.getMeta().getEnvVars().remove(RestConstants.JOB_ID);

Review comment:
       I don't quite understand why put the environment to the spec and then delete them?




----------------------------------------------------------------
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 #369: SUBMARINE-590. [WEB] Update chart/param/metric UI

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



##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
##########
@@ -85,13 +87,33 @@ private ExperimentManager(Submitter submitter) {
    */
   public Experiment createExperiment(ExperimentSpec spec) throws SubmarineRuntimeException {
     checkSpec(spec);
+
+    // Submarine sdk will get experimentID and JDBC URL from environment variables in each worker,
+    // and then log experiment metrics and parameters to submarine server
+    ExperimentId id = generateExperimentId();
+    String url = getSQLAlchemyURL();
+    spec.getMeta().getEnvVars().put(RestConstants.JOB_ID, id.toString());
+    spec.getMeta().getEnvVars().put(RestConstants.SUBMARINE_TRACKING_URI, url);
+
     Experiment experiment = submitter.createExperiment(spec);
-    experiment.setExperimentId(generateExperimentId());
+    experiment.setExperimentId(id);
+
+    spec.getMeta().getEnvVars().remove(RestConstants.JOB_ID);

Review comment:
       Those two environment variables are for internal usage, we set these environment variables in pods.
   But we would not like to see them in the experiment spec. 




----------------------------------------------------------------
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 #369: SUBMARINE-590. [WEB] Update chart/param/metric UI

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


   


----------------------------------------------------------------
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 pull request #369: SUBMARINE-590. [WEB] Update chart/param/metric UI

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


   @liuxunorg @jiwq Thanks for the review.
   I've updated the PR.


----------------------------------------------------------------
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] liuxunorg commented on pull request #369: SUBMARINE-590. [WEB] Update chart/param/metric UI

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


   @pingsutw Please fix the files conflict. Thanks.


----------------------------------------------------------------
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