Skip to content

Commit

Permalink
[bugfix] Fix value passing bug in New Experiment form (#2027)
Browse files Browse the repository at this point in the history
* [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
  • Loading branch information
orfeas-k authored Nov 23, 2022
1 parent d97c8ae commit 0a1cb31
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ ExperimentFormServiceStub = {
path: new FormControl(),
scheme: new FormControl(),
host: new FormControl(),
customYaml: new FormControl(),
}),
createTrialTemplateForm: () =>
new FormGroup({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
<ng-container *ngSwitchCase="kind.CUSTOM">
<lib-monaco-editor
[(text)]="customYaml"
(textChange)="onTextChange()"
[language]="'yaml'"
[readOnly]="false"
[width]="375"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('FormMetricsCollectorComponent', () => {
scheme: new FormControl(),
host: new FormControl(),
prometheus: new FormControl(),
customYaml: new FormControl(),
});
fixture.detectChanges();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: <collector-image>\nresources: {}';
customYaml: string;

constructor() {}

ngOnInit() {}
ngOnInit() {
this.customYaml = this.formGroup.get('customYaml').value;
}

onTextChange() {
this.formGroup.patchValue({ customYaml: this.customYaml });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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: {} },
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -17,12 +18,17 @@ export class YamlModalComponent {
constructor(
public dialogRef: MatDialogRef<YamlModalComponent>,
@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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ export class ExperimentFormService {
host: this.builder.control(''),
httpHeaders: this.builder.array([]),
}),
customYaml:
'name: metrics-collector\nimage: <collector-image>\nresources: {}',
});
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 0a1cb31

Please sign in to comment.