-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[Retiarii] Visualization #3878
[Retiarii] Visualization #3878
Changes from 12 commits
c140ca0
d3c5d86
e108fbb
564bd7c
0713220
7f10d32
5d2b3fd
d3888cd
57f01d8
b7e133e
76e68e4
bce45ca
fc636d6
650a5fb
7e3dc63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,3 +10,4 @@ _generated_model | |
data | ||
generated | ||
lightning_logs | ||
model.onnx |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just removed LogType as I think it's no longer used. |
||
|
||
interface TrainingServiceMetadata { | ||
readonly key: string; | ||
|
@@ -81,7 +81,7 @@ abstract class TrainingService { | |
public abstract submitTrialJob(form: TrialJobApplicationForm): Promise<TrialJobDetail>; | ||
public abstract updateTrialJob(trialJobId: string, form: TrialJobApplicationForm): Promise<TrialJobDetail>; | ||
public abstract cancelTrialJob(trialJobId: string, isEarlyStopped?: boolean): Promise<void>; | ||
public abstract getTrialLog(trialJobId: string, logType: LogType): Promise<string>; | ||
public abstract getTrialFile(trialJobId: string, fileName: string): Promise<Buffer | string>; | ||
public abstract setClusterMetadata(key: string, value: string): Promise<void>; | ||
public abstract getClusterMetadata(key: string): Promise<string>; | ||
public abstract getTrialOutputLocalPath(trialJobId: string): Promise<string>; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not import? type definition? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idk. Copied code. I'll try import. |
||
|
||
/** | ||
* NNI Main rest server, provides rest API to support | ||
* # nnictl CLI tool | ||
|
@@ -21,6 +23,7 @@ import { getAPIRootUrl } from '../common/experimentStartupInfo'; | |
@component.Singleton | ||
export class NNIRestServer extends RestServer { | ||
private readonly LOGS_ROOT_URL: string = '/logs'; | ||
protected netronProxy: any = null; | ||
protected API_ROOT_URL: string = '/api/v1/nni'; | ||
|
||
/** | ||
|
@@ -29,6 +32,7 @@ export class NNIRestServer extends RestServer { | |
constructor() { | ||
super(); | ||
this.API_ROOT_URL = getAPIRootUrl(); | ||
this.netronProxy = httpProxy.createProxyServer(); | ||
} | ||
|
||
/** | ||
|
@@ -39,6 +43,14 @@ export class NNIRestServer extends RestServer { | |
this.app.use(bodyParser.json({limit: '50mb'})); | ||
this.app.use(this.API_ROOT_URL, createRestHandler(this)); | ||
this.app.use(this.LOGS_ROOT_URL, express.static(getLogDir())); | ||
this.app.all('/netron/*', (req: express.Request, res: express.Response) => { | ||
delete req.headers.host; | ||
req.url = req.url.replace('/netron', '/'); | ||
this.netronProxy.web(req, res, { | ||
changeOrigin: true, | ||
target: 'https://netron.app' | ||
}); | ||
}); | ||
this.app.get('*', (req: express.Request, res: express.Response) => { | ||
res.sendFile(path.resolve('static/index.html')); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,7 @@ class NNIRestHandler { | |
this.version(router); | ||
this.checkStatus(router); | ||
this.getExperimentProfile(router); | ||
this.getExperimentMetadata(router); | ||
this.updateExperimentProfile(router); | ||
this.importData(router); | ||
this.getImportedData(router); | ||
|
@@ -66,7 +67,7 @@ class NNIRestHandler { | |
this.getMetricData(router); | ||
this.getMetricDataByRange(router); | ||
this.getLatestMetricData(router); | ||
this.getTrialLog(router); | ||
this.getTrialFile(router); | ||
this.exportData(router); | ||
this.getExperimentsInfo(router); | ||
this.startTensorboardTask(router); | ||
|
@@ -296,13 +297,20 @@ class NNIRestHandler { | |
}); | ||
} | ||
|
||
private getTrialLog(router: Router): void { | ||
router.get('/trial-log/:id/:type', async(req: Request, res: Response) => { | ||
this.nniManager.getTrialLog(req.params.id, req.params.type as LogType).then((log: string) => { | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems ut in |
||
let encoding: string | null = null; | ||
const filename = req.params.filename; | ||
if (!filename.includes('.') || filename.match(/.*\.(txt|log)/g)) { | ||
encoding = 'utf8'; | ||
} | ||
this.nniManager.getTrialFile(req.params.id, filename).then((content: Buffer | string) => { | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
res.send(log); | ||
res.send(content); | ||
}).catch((err: Error) => { | ||
this.handleError(err, res); | ||
}); | ||
|
@@ -319,6 +327,24 @@ class NNIRestHandler { | |
}); | ||
} | ||
|
||
private getExperimentMetadata(router: Router): void { | ||
router.get('/experiment-metadata', (req: Request, res: Response) => { | ||
Promise.all([ | ||
this.nniManager.getExperimentProfile(), | ||
this.experimentsManager.getExperimentsInfo() | ||
]).then(([profile, experimentInfo]) => { | ||
for (const info of experimentInfo as any) { | ||
if (info.id === profile.id) { | ||
res.send(info); | ||
break; | ||
} | ||
} | ||
}).catch((err: Error) => { | ||
this.handleError(err, res); | ||
}); | ||
}); | ||
} | ||
|
||
private getExperimentsInfo(router: Router): void { | ||
router.get('/experiments-info', (req: Request, res: Response) => { | ||
this.experimentsManager.getExperimentsInfo().then((experimentInfo: JSON) => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a broken change