From 0a1cb313642a4fe6bdd72fd6031fca8cc0b43d6c Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Wed, 23 Nov 2022 15:48:43 +0200 Subject: [PATCH] [bugfix] Fix value passing bug in New Experiment form (#2027) * [bugfix] Fix value passing bug in New Experiment form Add missing logic in New Experiment form in order to pass the value of the editor content in Metrics Collector tab, when Kind is set to Custom. * Adjust unit tests for custom yaml metrics collector --- .../experiment-creation.component.spec.ts | 1 + .../metrics-collector.component.html | 1 + .../metrics-collector.component.spec.ts | 1 + .../metrics-collector.component.ts | 11 ++++++++--- .../yaml-modal/yaml-modal.component.spec.ts | 9 ++++++++- .../yaml-modal/yaml-modal.component.ts | 8 +++++++- .../app/services/experiment-form.service.ts | 19 +++++++++++++++++-- 7 files changed, 43 insertions(+), 7 deletions(-) diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/experiment-creation.component.spec.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/experiment-creation.component.spec.ts index 85c2b100598..c29716a95bd 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/experiment-creation.component.spec.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/experiment-creation.component.spec.ts @@ -81,6 +81,7 @@ ExperimentFormServiceStub = { path: new FormControl(), scheme: new FormControl(), host: new FormControl(), + customYaml: new FormControl(), }), createTrialTemplateForm: () => new FormGroup({ diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.html b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.html index 06a90f6b1b6..4addd8cf258 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.html +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.html @@ -70,6 +70,7 @@ { scheme: new FormControl(), host: new FormControl(), prometheus: new FormControl(), + customYaml: new FormControl(), }); fixture.detectChanges(); }); diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.ts index e8cf704cda6..779758d640c 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/metrics-collector/metrics-collector.component.ts @@ -10,10 +10,15 @@ import { CollectorKind } from 'src/app/enumerations/metrics-collector'; export class FormMetricsCollectorComponent implements OnInit { @Input() formGroup: FormGroup; kind = CollectorKind; - customYaml = - 'name: metrics-collector\nimage: \nresources: {}'; + customYaml: string; constructor() {} - ngOnInit() {} + ngOnInit() { + this.customYaml = this.formGroup.get('customYaml').value; + } + + onTextChange() { + this.formGroup.patchValue({ customYaml: this.customYaml }); + } } diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.spec.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.spec.ts index 721e0f49073..ccaba4eff81 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.spec.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.spec.ts @@ -2,6 +2,7 @@ import { CommonModule } from '@angular/common'; import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { MatDialogRef, MAT_DIALOG_DATA } from '@angular/material/dialog'; import { MatDialogModule } from '@angular/material/dialog'; +import { MatSnackBarModule } from '@angular/material/snack-bar'; import { EditorModule, FormModule } from 'kubeflow'; import { YamlModalComponent } from './yaml-modal.component'; @@ -13,7 +14,13 @@ describe('YamlModalComponent', () => { beforeEach( waitForAsync(() => { TestBed.configureTestingModule({ - imports: [CommonModule, MatDialogModule, FormModule, EditorModule], + imports: [ + CommonModule, + MatDialogModule, + FormModule, + EditorModule, + MatSnackBarModule, + ], declarations: [YamlModalComponent], providers: [ { provide: MatDialogRef, useValue: {} }, diff --git a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.ts b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.ts index f4046ae9f38..dfdc10d91cb 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/pages/experiment-creation/yaml-modal/yaml-modal.component.ts @@ -1,5 +1,6 @@ import { Component, Input, Inject } from '@angular/core'; import { load, dump } from 'js-yaml'; +import { SnackBarService, SnackType } from 'kubeflow'; import { MatDialog, MatDialogRef, @@ -17,12 +18,17 @@ export class YamlModalComponent { constructor( public dialogRef: MatDialogRef, @Inject(MAT_DIALOG_DATA) public data: any, + private snack: SnackBarService, ) { this.yaml = dump(data); } save() { - this.dialogRef.close(load(this.yaml)); + try { + this.dialogRef.close(load(this.yaml)); + } catch (e) { + this.snack.open(`${e.reason}`, SnackType.Error, 4000); + } } close() { diff --git a/pkg/new-ui/v1beta1/frontend/src/app/services/experiment-form.service.ts b/pkg/new-ui/v1beta1/frontend/src/app/services/experiment-form.service.ts index 91f876fe3c5..08dedba7236 100644 --- a/pkg/new-ui/v1beta1/frontend/src/app/services/experiment-form.service.ts +++ b/pkg/new-ui/v1beta1/frontend/src/app/services/experiment-form.service.ts @@ -207,6 +207,8 @@ export class ExperimentFormService { host: this.builder.control(''), httpHeaders: this.builder.array([]), }), + customYaml: + 'name: metrics-collector\nimage: \nresources: {}', }); } @@ -391,8 +393,17 @@ export class ExperimentFormService { return metrics; } - /* TODO(kimwnasptd): We need to handle the Custom case */ if (kind === 'Custom') { + delete metrics.source.fileSystemPath; + try { + metrics.collector.customCollector = load(group.get('customYaml').value); + } catch (e) { + this.snack.open( + 'Metrics Colletor(Custom): ' + `${e.reason}`, + SnackType.Error, + 4000, + ); + } return metrics; } @@ -421,7 +432,11 @@ export class ExperimentFormService { try { trialTemplate.trialSpec = load(formValue.yaml); } catch (e) { - this.snack.open(`${e.reason}`, SnackType.Warning, 4000); + this.snack.open( + 'Trial Template: ' + `${e.reason}`, + SnackType.Error, + 4000, + ); } return trialTemplate;