Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Support space in logDir #1694

Merged
merged 10 commits into from
Nov 11, 2019
Merged

Support space in logDir #1694

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
80ccfcc
upload code dir seperately
SparkSnail Jul 16, 2019
c3aec18
fix frameworkcontroller
SparkSnail Jul 16, 2019
9915446
remove unused code
SparkSnail Jul 16, 2019
ffed089
fix comments
SparkSnail Jul 22, 2019
037e763
fix conflict
SparkSnail Nov 3, 2019
2c92ad9
init
SparkSnail Nov 3, 2019
de89a89
fix comments
SparkSnail Nov 4, 2019
38372b9
fix windows quota
SparkSnail Nov 4, 2019
1c01229
revert change
SparkSnail Nov 4, 2019
852ebdd
fix quota
SparkSnail Nov 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/nni_manager/common/experimentStartupInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class ExperimentStartupInfo {
this.initialized = true;

if (logDir !== undefined && logDir.length > 0) {
this.logDir = path.join(logDir, getExperimentId());
this.logDir = path.join(path.normalize(logDir), getExperimentId());
} else {
this.logDir = path.join(os.homedir(), 'nni', 'experiments', getExperimentId());
}
Expand Down
26 changes: 13 additions & 13 deletions src/nni_manager/training_service/common/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ export async function validateCodeDir(codeDir: string) : Promise<number> {
*/
export async function execMkdir(directory: string, share: boolean = false): Promise<void> {
if (process.platform === 'win32') {
await cpp.exec(`powershell.exe New-Item -Path ${directory} -ItemType "directory" -Force`);
await cpp.exec(`powershell.exe New-Item -Path '${directory}' -ItemType "directory" -Force`);
} else if (share) {
await cpp.exec(`(umask 0; mkdir -p ${directory})`);
await cpp.exec(`(umask 0; mkdir -p '${directory}')`);
} else {
await cpp.exec(`mkdir -p ${directory}`);
await cpp.exec(`mkdir -p '${directory}'`);
}

return Promise.resolve();
Expand All @@ -87,9 +87,9 @@ export async function execMkdir(directory: string, share: boolean = false): Prom
*/
export async function execCopydir(source: string, destination: string): Promise<void> {
if (process.platform === 'win32') {
await cpp.exec(`powershell.exe Copy-Item ${source} -Destination ${destination} -Recurse`);
await cpp.exec(`powershell.exe Copy-Item '${source}' -Destination '${destination}' -Recurse`);
} else {
await cpp.exec(`cp -r ${source} ${destination}`);
await cpp.exec(`cp -r '${source}' '${destination}'`);
}

return Promise.resolve();
Expand All @@ -101,9 +101,9 @@ export async function execCopydir(source: string, destination: string): Promise<
*/
export async function execNewFile(filename: string): Promise<void> {
if (process.platform === 'win32') {
await cpp.exec(`powershell.exe New-Item -Path ${filename} -ItemType "file" -Force`);
await cpp.exec(`powershell.exe New-Item -Path '${filename}' -ItemType "file" -Force`);
} else {
await cpp.exec(`touch ${filename}`);
await cpp.exec(`touch '${filename}'`);
}

return Promise.resolve();
Expand All @@ -115,9 +115,9 @@ export async function execNewFile(filename: string): Promise<void> {
*/
export function runScript(filePath: string): cp.ChildProcess {
if (process.platform === 'win32') {
return cp.exec(`powershell.exe -ExecutionPolicy Bypass -file ${filePath}`);
return cp.exec(`powershell.exe -ExecutionPolicy Bypass -file '${filePath}'`);
} else {
return cp.exec(`bash ${filePath}`);
return cp.exec(`bash '${filePath}'`);
}
}

Expand All @@ -128,9 +128,9 @@ export function runScript(filePath: string): cp.ChildProcess {
export async function execTail(filePath: string): Promise<cpp.childProcessPromise.Result> {
let cmdresult: cpp.childProcessPromise.Result;
if (process.platform === 'win32') {
cmdresult = await cpp.exec(`powershell.exe Get-Content ${filePath} -Tail 1`);
cmdresult = await cpp.exec(`powershell.exe Get-Content '${filePath}' -Tail 1`);
} else {
cmdresult = await cpp.exec(`tail -n 1 ${filePath}`);
cmdresult = await cpp.exec(`tail -n 1 '${filePath}'`);
}

return Promise.resolve(cmdresult);
Expand All @@ -142,9 +142,9 @@ export async function execTail(filePath: string): Promise<cpp.childProcessPromis
*/
export async function execRemove(directory: string): Promise<void> {
if (process.platform === 'win32') {
await cpp.exec(`powershell.exe Remove-Item ${directory} -Recurse -Force`);
await cpp.exec(`powershell.exe Remove-Item '${directory}' -Recurse -Force`);
} else {
await cpp.exec(`rm -rf ${directory}`);
await cpp.exec(`rm -rf '${directory}'`);
}

return Promise.resolve();
Expand Down
16 changes: 8 additions & 8 deletions src/nni_manager/training_service/local/localTrainingService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,9 @@ class LocalTrainingService implements TrainingService {
const envVariables: { key: string; value: string }[] = [
{ key: 'NNI_PLATFORM', value: 'local' },
{ key: 'NNI_EXP_ID', value: this.experimentId },
{ key: 'NNI_SYS_DIR', value: trialJobDetail.workingDirectory },
{ key: 'NNI_SYS_DIR', value: `'${trialJobDetail.workingDirectory}'` },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the value of environment variables should to be quoted.
We should quote them when composing the command, not here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

{ key: 'NNI_TRIAL_JOB_ID', value: trialJobDetail.id },
{ key: 'NNI_OUTPUT_DIR', value: trialJobDetail.workingDirectory },
{ key: 'NNI_OUTPUT_DIR', value: `'${trialJobDetail.workingDirectory}'` },
{ key: 'NNI_TRIAL_SEQ_ID', value: trialJobDetail.form.sequenceId.toString() },
{ key: 'MULTI_PHASE', value: this.isMultiPhase.toString() }
];
Expand Down Expand Up @@ -490,18 +490,18 @@ class LocalTrainingService implements TrainingService {
const script: string[] = [];
if (process.platform === 'win32') {
script.push(
`cmd.exe /c ${localTrialConfig.command} 2>${path.join(workingDirectory, 'stderr')}`,
`cmd.exe /c ${localTrialConfig.command} 2>'${path.join(workingDirectory, 'stderr')}'`,
`$NOW_DATE = [int64](([datetime]::UtcNow)-(get-date "1/1/1970")).TotalSeconds`,
`$NOW_DATE = "$NOW_DATE" + (Get-Date -Format fff).ToString()`,
`Write $LASTEXITCODE " " $NOW_DATE | Out-File ${path.join(workingDirectory, '.nni', 'state')} -NoNewline -encoding utf8`);
`Write $LASTEXITCODE " " $NOW_DATE | Out-File '${path.join(workingDirectory, '.nni', 'state')}' -NoNewline -encoding utf8`);
} else {
script.push(`eval ${localTrialConfig.command} 2>${path.join(workingDirectory, 'stderr')}`);
script.push(`eval ${localTrialConfig.command} 2>'${path.join(workingDirectory, 'stderr')}'`);
if (process.platform === 'darwin') {
// https://superuser.com/questions/599072/how-to-get-bash-execution-time-in-milliseconds-under-mac-os-x
// Considering the worst case, write 999 to avoid negative duration
script.push(`echo $? \`date +%s999\` >${path.join(workingDirectory, '.nni', 'state')}`);
script.push(`echo $? \`date +%s999\` >'${path.join(workingDirectory, '.nni', 'state')}'`);
} else {
script.push(`echo $? \`date +%s%3N\` >${path.join(workingDirectory, '.nni', 'state')}`);
script.push(`echo $? \`date +%s%3N\` >'${path.join(workingDirectory, '.nni', 'state')}'`);
}
}

Expand All @@ -522,7 +522,7 @@ class LocalTrainingService implements TrainingService {
if (process.platform !== 'win32') {
runScriptContent.push('#!/bin/bash');
}
runScriptContent.push(`cd ${this.localTrialConfig.codeDir}`);
runScriptContent.push(`cd '${this.localTrialConfig.codeDir}'`);
for (const variable of variables) {
runScriptContent.push(setEnvironmentVariable(variable));
}
Expand Down