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 2021/08/01 06:58:42 UTC

[GitHub] [submarine] noidname01 opened a new pull request #692: SUBMARINE-955. Add Limit on Experiment Name

noidname01 opened a new pull request #692:
URL: https://github.com/apache/submarine/pull/692


   ### What is this PR for?
   
   [Former PR](https://issues.apache.org/jira/browse/SUBMARINE-886) has some edge case.
   Like:
   * \-
   * --invalid
   
   Valid cases are like:
   * valid-one
   * valid1
   
   So I change the pattern for validating experiment name to filter these cases.
   And I also add this pattern to experiment name of predefine template.
   
   ### What type of PR is it?
   [Bug Fix]
   
   ### Todos
   
   None
   
   ### What is the Jira issue?
   
   https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-955
   
   ### How should this be tested?
   
   Open the workbench and type invalid experiment name.
   
   ### Screenshots (if appropriate)
   
   invalid name on custom experiment
   ![2021-08-01 14-47-13 的螢幕擷圖](https://user-images.githubusercontent.com/55401762/127762058-e73ad41d-546b-4b45-960a-5d15b646d2cc.png)
   invalid name on predefine experiment
   ![2021-08-01 14-47-34 的螢幕擷圖](https://user-images.githubusercontent.com/55401762/127762063-1510f983-a388-4472-89c4-7b7451cbaab5.png)
   valid name on custom experiment
   ![2021-08-01 14-48-07 的螢幕擷圖](https://user-images.githubusercontent.com/55401762/127762066-2816e348-3202-43ce-b541-41ab4c85ca1b.png)
   valid name on predefine experiment
   ![2021-08-01 14-47-52 的螢幕擷圖](https://user-images.githubusercontent.com/55401762/127762067-eda0d8c6-cb71-426c-a8af-6f446e9bffa8.png)
   
   
   ### Questions:
   * Do the license files need updating? No
   * Are there breaking changes for older versions? No
   * Does this need new 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.

To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org

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

[GitHub] [submarine] kevin85421 commented on a change in pull request #692: SUBMARINE-955. Add Limit on Experiment Name

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



##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-predefined-form/experiment-predefined-form.component.ts
##########
@@ -151,8 +151,11 @@ export class ExperimentPredefinedFormComponent implements OnInit, OnDestroy {
     let controls = {};
     for (let item of this.templates[this.currentOption].templateParams) {
       controls[item.name] = [item.value];
-      if (item.required === 'true') controls[item.name].push([Validators.required]);
+      if (item.required === 'true') {
+        if(item.name !== 'experiment_name') controls[item.name].push([Validators.required]);
+        else controls[item.name].push([Validators.required, Validators.pattern('[a-zA-Z0-9]+[a-zA-Z0-9\-]*')]);
     }

Review comment:
       indentation

##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-customized-form/experiment-customized-form.component.html
##########
@@ -42,7 +42,7 @@
             placeholder="mnist-example"
           />
           <div class="alert-message" *ngIf="experiment.get('experimentName').hasError('pattern')">
-            Only letters(a-z), numbers(0-9), and hyphens(-) are allowed.
+            Only letters(a-z), numbers(0-9), and hyphens(-)(but cannot start with them) are allowed.

Review comment:
       We can discuss that should we need to change the error message in today's meeting. 

##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-predefined-form/experiment-predefined-form.component.scss
##########
@@ -16,3 +16,9 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+
+ .alert-message {
+    color: red;
+    margin-top: .3rem;
+    line-height: normal;
+   }

Review comment:
       indentation

##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-predefined-form/experiment-predefined-form.component.ts
##########
@@ -151,8 +151,11 @@ export class ExperimentPredefinedFormComponent implements OnInit, OnDestroy {
     let controls = {};
     for (let item of this.templates[this.currentOption].templateParams) {
       controls[item.name] = [item.value];
-      if (item.required === 'true') controls[item.name].push([Validators.required]);
+      if (item.required === 'true') {
+        if(item.name !== 'experiment_name') controls[item.name].push([Validators.required]);
+        else controls[item.name].push([Validators.required, Validators.pattern('[a-zA-Z0-9]+[a-zA-Z0-9\-]*')]);
     }
+  }

Review comment:
       indentation




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

To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org

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

[GitHub] [submarine] kevin85421 commented on a change in pull request #692: SUBMARINE-955. Add Limit on Experiment Name

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



##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-predefined-form/experiment-predefined-form.component.ts
##########
@@ -151,8 +151,11 @@ export class ExperimentPredefinedFormComponent implements OnInit, OnDestroy {
     let controls = {};
     for (let item of this.templates[this.currentOption].templateParams) {
       controls[item.name] = [item.value];
-      if (item.required === 'true') controls[item.name].push([Validators.required]);
+      if (item.required === 'true') {
+        if(item.name !== 'experiment_name') controls[item.name].push([Validators.required]);
+        else controls[item.name].push([Validators.required, Validators.pattern('[a-zA-Z0-9]+[a-zA-Z0-9\-]*')]);
     }

Review comment:
       indentation

##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-customized-form/experiment-customized-form.component.html
##########
@@ -42,7 +42,7 @@
             placeholder="mnist-example"
           />
           <div class="alert-message" *ngIf="experiment.get('experimentName').hasError('pattern')">
-            Only letters(a-z), numbers(0-9), and hyphens(-) are allowed.
+            Only letters(a-z), numbers(0-9), and hyphens(-)(but cannot start with them) are allowed.

Review comment:
       We can discuss that should we need to change the error message in today's meeting. 

##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-predefined-form/experiment-predefined-form.component.scss
##########
@@ -16,3 +16,9 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+
+ .alert-message {
+    color: red;
+    margin-top: .3rem;
+    line-height: normal;
+   }

Review comment:
       indentation

##########
File path: submarine-workbench/workbench-web/src/app/pages/workbench/experiment/experiment-home/experiment-form/experiment-predefined-form/experiment-predefined-form.component.ts
##########
@@ -151,8 +151,11 @@ export class ExperimentPredefinedFormComponent implements OnInit, OnDestroy {
     let controls = {};
     for (let item of this.templates[this.currentOption].templateParams) {
       controls[item.name] = [item.value];
-      if (item.required === 'true') controls[item.name].push([Validators.required]);
+      if (item.required === 'true') {
+        if(item.name !== 'experiment_name') controls[item.name].push([Validators.required]);
+        else controls[item.name].push([Validators.required, Validators.pattern('[a-zA-Z0-9]+[a-zA-Z0-9\-]*')]);
     }
+  }

Review comment:
       indentation




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

To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org

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

[GitHub] [submarine] asfgit closed pull request #692: SUBMARINE-955. Add Limit on Experiment Name

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


   


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

To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org

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