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/12/07 12:53:07 UTC

[GitHub] [submarine] ByronHsu opened a new pull request #470: Submarine 690. Enable pre-defined web interface in submarine workbench

ByronHsu opened a new pull request #470:
URL: https://github.com/apache/submarine/pull/470


   ### What is this PR for?
   Add pre-defined web interface.
   The steps of users to create a pre-defined experiment are as following:
   1. Click pre-defined button
   2. Pre-defined data will be fetched from db
   3. Render configurable params and spec on ng-modal
   4. When the form is validated, the experiment can be submitted.
   
   
   ### What type of PR is it?
   [Feature]
   
   ### Todos
   - [ ] Test predefined component (The e2e test failed now due to unknown reason)
   - [ ] Real-time fetch experiment status
   - [ ] The implementation logic of the experiment page should be improved (e.g. split components correctly, use resolver to avoid long-time waiting of data)
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-690
   
   ### How should this be tested?
   https://github.com/ByronHsu/submarine/actions/runs/404158134
   
   ### Screenshots (if appropriate)
   ![Kapture 2020-12-07 at 17 18 34](https://user-images.githubusercontent.com/24364830/101353091-04360680-38ce-11eb-9ccd-89d383d09d99.gif)
   
   


----------------------------------------------------------------
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] ByronHsu commented on pull request #470: Submarine 690. Enable pre-defined web interface in submarine workbench

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


   @tangzhankun @pingsutw @xunliu 
   Please help me to review this PR. 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



[GitHub] [submarine] asfgit closed pull request #470: SUBMARINE-690. Enable pre-defined web interface in submarine workbench

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


   


----------------------------------------------------------------
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 #470: Submarine 690. Enable pre-defined web interface in submarine workbench

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



##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-home.component.ts
##########
@@ -7,7 +7,7 @@
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
  *
- *   http://www.apache.org/licenses/LICENSE-2.0
+ *   http: //www.apache.org/licenses/LICENSE-2.0

Review comment:
       This space is not needed actually. Maybe some settings in your IDE cause this?




----------------------------------------------------------------
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 #470: SUBMARINE-690. Enable pre-defined web interface in submarine workbench

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



##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment.component.ts
##########
@@ -101,7 +101,7 @@ export class ExperimentComponent implements OnInit {
       this.modalProps = { ...this.modalProps, ...props };
     });
 
-    this.reloadCheck();
+    // this.reloadCheck();

Review comment:
       Why comment out this line?




----------------------------------------------------------------
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 #470: SUBMARINE-690. Enable pre-defined web interface in submarine workbench

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



##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-predefined-form/experiment-predefined-form.component.ts
##########
@@ -0,0 +1,171 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Component, OnDestroy, OnInit } from '@angular/core';
+import { FormBuilder, FormGroup, Validators } from '@angular/forms';
+import { ExperimentTemplate } from '@submarine/interfaces/experiment-template';
+import { ExperimentFormService } from '@submarine/services/experiment.form.service';
+import { ExperimentService } from '@submarine/services/experiment.service';
+import { interval, Subscription } from 'rxjs';
+import { ExperimentTemplateSubmit } from '@submarine/interfaces/experiment-template-submit';
+import { NzMessageService } from 'ng-zorro-antd';
+
+interface ParsedTemplate {
+  templateParams: {
+    name: string;
+    required: string;
+    description: string;
+    value: string;
+  }[];
+  experimentName: string;
+  experimentNamespace: string;
+  experimentCommand: string;
+  experimentImage: string;
+  experimentVars: string;
+}
+
+interface TemplateTable {
+  [templateName: string]: ParsedTemplate;
+}
+
+@Component({
+  selector: 'submarine-experiment-predefined-form',
+  templateUrl: './experiment-predefined-form.component.html',
+  styleUrls: ['./experiment-predefined-form.component.scss'],
+})
+export class ExperimentPredefinedFormComponent implements OnInit, OnDestroy {
+  /* states that are bond to html template */
+  paramList: { name: string; required: string }[];
+
+  /* inner states */
+  templates: TemplateTable = {};
+  predefinedForm: FormGroup;
+  subs: Subscription[] = [];
+
+  constructor(
+    private experimentService: ExperimentService,
+    private experimentFormService: ExperimentFormService,
+    private fb: FormBuilder,
+    private nzMessageService: NzMessageService
+  ) {}
+
+  ngOnInit() {
+    this.experimentService.fetchExperimentTemplateList().subscribe((res) => {
+      this.templates = this.parseTemplateRespond(res);
+
+      if (Object.keys(this.templates).length != 0) {
+        // default: switch to option 1

Review comment:
       ```suggestion
           // default: switch to first template
   ```




----------------------------------------------------------------
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 #470: Submarine 690. Enable pre-defined web interface in submarine workbench

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


   I will review as soon as possible~


----------------------------------------------------------------
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] ByronHsu commented on a change in pull request #470: SUBMARINE-690. Enable pre-defined web interface in submarine workbench

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



##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment.component.ts
##########
@@ -101,7 +101,7 @@ export class ExperimentComponent implements OnInit {
       this.modalProps = { ...this.modalProps, ...props };
     });
 
-    this.reloadCheck();
+    // this.reloadCheck();

Review comment:
       Oh, I accidentally commented on this line. 
   However, I think there is a better way to handle this issue. I will check it later when I start to refactor the code.




----------------------------------------------------------------
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] ByronHsu commented on a change in pull request #470: SUBMARINE-690. Enable pre-defined web interface in submarine workbench

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



##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-home.component.ts
##########
@@ -7,7 +7,7 @@
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
  *
- *   http://www.apache.org/licenses/LICENSE-2.0
+ *   http: //www.apache.org/licenses/LICENSE-2.0

Review comment:
       Thanks. I've fixed it.




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