[Retiarii] Visualization #3878
[Retiarii] Visualization #3878
Conversation
| updated ||= !compareProfiles(this.profileField, profile); | ||
| this.profileField = profile; | ||
|
|
||
| updated ||= JSON.stringify(this.metadataField) !== JSON.stringify(metadata); |
There was a problem hiding this comment.
need to update page when metadata update?
There was a problem hiding this comment.
I'm not sure. Removing it for now.
There was a problem hiding this comment.
OK. I could test it during the bug bash.
| @@ -8,7 +8,7 @@ | |||
| */ | |||
| type TrialJobStatus = 'UNKNOWN' | 'WAITING' | 'RUNNING' | 'SUCCEEDED' | 'FAILED' | 'USER_CANCELED' | 'SYS_CANCELED' | 'EARLY_STOPPED'; | |||
|
|
|||
| type LogType = 'TRIAL_LOG' | 'TRIAL_STDOUT' | 'TRIAL_ERROR'; | |||
| type LogType = 'TRIAL_LOG' | 'TRIAL_STDOUT' | 'TRIAL_ERROR' | 'MODEL.onnx'; | |||
There was a problem hiding this comment.
We can remove it if the parameter type is 'string' or keep this type for parameter and rename it as "FileName"?
There was a problem hiding this comment.
Just removed LogType as I think it's no longer used.
| @@ -12,6 +12,8 @@ import { getLogDir } from '../common/utils'; | |||
| import { createRestHandler } from './restHandler'; | |||
| import { getAPIRootUrl } from '../common/experimentStartupInfo'; | |||
|
|
|||
| const httpProxy = require('http-proxy'); | |||
There was a problem hiding this comment.
Why not import? type definition?
There was a problem hiding this comment.
idk. Copied code. I'll try import.
| if (log === '') { | ||
| log = 'No logs available.' | ||
| private getTrialFile(router: Router): void { | ||
| router.get('/trial-file/:id/:filename', async(req: Request, res: Response) => { |
There was a problem hiding this comment.
seems ut in restful_server.py use /trial-log/:id/:type
| if (content instanceof Buffer) { | ||
| res.header('Content-Type', 'application/octet-stream'); | ||
| } else if (content === '') { | ||
| content = 'No logs available.' |
There was a problem hiding this comment.
No logs available. seems strange since it is getTrialFile() function.
| throw new Error(`File unaccessible: ${fileName}`); | ||
| } | ||
| let encoding: string | null = null; | ||
| if (!fileName.includes('.') || fileName.match(/.*\.(txt|log)/g)) { |
There was a problem hiding this comment.
I suggest rename stdout to stdout.txt...
Not the responsibility of this pr though...
| updated ||= !compareProfiles(this.profileField, profile); | ||
| this.profileField = profile; | ||
|
|
||
| if (JSON.stringify(this.metadataField) !== JSON.stringify(metadata)) { |
There was a problem hiding this comment.
Seems this if is meaningless
| @@ -59,7 +59,7 @@ abstract class Manager { | |||
| public abstract getMetricDataByRange(minSeqId: number, maxSeqId: number): Promise<MetricDataRecord[]>; | |||
| public abstract getLatestMetricData(): Promise<MetricDataRecord[]>; | |||
|
|
|||
| public abstract getTrialLog(trialJobId: string, logType: LogType): Promise<string>; | |||
| public abstract getTrialFile(trialJobId: string, fileName: string): Promise<Buffer | string>; | |||
There was a problem hiding this comment.
The behavior would be more predictable to split into get text and get binary.
If you have time, not mandatory...
There was a problem hiding this comment.
I'm having a hard time thinking of another meaningful name.
Let's do it in future when we actually need this one.
There was a problem hiding this comment.
this is a broken change
This is the first working example of Retiarii visualization.
TODOs: