Skip to content

Commit

Permalink
refactor: update eslint rules to remove warnings during build (micros…
Browse files Browse the repository at this point in the history
…oft#2405)

update nni manager eslint rules.

1. add rule to ignore unused args, and treat other unused as error.
2. add run to treat no explicit function return type as error.
3. fix all errors.
  • Loading branch information
squirrelsc authored May 7, 2020
1 parent f67cebd commit 6485ef1
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 87 deletions.
7 changes: 7 additions & 0 deletions src/nni_manager/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@
"@typescript-eslint/consistent-type-assertions": 0,
"@typescript-eslint/no-inferrable-types": 0,
"no-inner-declarations": 0,
"@typescript-eslint/explicit-function-return-type": "error",
"@typescript-eslint/no-unused-vars": [
"error",
{
"argsIgnorePattern": "^_"
}
],
"@typescript-eslint/no-var-requires": 0
},
"ignorePatterns": [
Expand Down
2 changes: 1 addition & 1 deletion src/nni_manager/core/sqlDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class SqlDB implements Database {
this.resolve(this.initTask, err);
} else {
if (createNew) {
this.db.exec(createTables, (error: Error | null) => {
this.db.exec(createTables, (_error: Error | null) => {
this.resolve(this.initTask, err);
});
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/nni_manager/rest_server/restHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class NNIRestHandler {
this.exportData(router);

// Express-joi-validator configuration
router.use((err: any, req: Request, res: Response, next: any) => {
router.use((err: any, _req: Request, res: Response, _next: any) => {
if (err.isBoom) {
this.log.error(err.output.payload);

Expand Down
6 changes: 3 additions & 3 deletions src/nni_manager/training_service/dlts/dltsTrainingService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class DLTSTrainingService implements TrainingService {
private async statusCheckingLoop(): Promise<void> {
while (!this.stopping) {
const updateDLTSTrialJobs: Promise<void>[] = [];
for (const [trialJobId, dltsTrialJob] of this.trialJobsMap) {
for (const dltsTrialJob of this.trialJobsMap.values()) {
updateDLTSTrialJobs.push(this.getDLTSTrialJobInfo(dltsTrialJob));
}

Expand Down Expand Up @@ -405,7 +405,7 @@ class DLTSTrainingService implements TrainingService {
}
}

public async getClusterMetadata(key: string): Promise<string> {
public async getClusterMetadata(_key: string): Promise<string> {
return '';
}

Expand Down Expand Up @@ -545,7 +545,7 @@ class DLTSTrainingService implements TrainingService {
body: parameterFileMeta
};
await new Promise((resolve, reject) => {
request(req, (err: Error, res: request.Response) => {
request(req, (err: Error, _res: request.Response) => {
if (err) {
reject(err);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export namespace AzureStorageClientUtility {
*/
export async function createShare(fileServerClient: any, azureShare: any): Promise<boolean> {
const deferred: Deferred<boolean> = new Deferred<boolean>();
fileServerClient.createShareIfNotExists(azureShare, (error: any, result: any, response: any) => {
fileServerClient.createShareIfNotExists(azureShare, (error: any, _result: any, _response: any) => {
if (error) {
getLogger()
.error(`Create share failed:, ${error}`);
Expand All @@ -41,7 +41,7 @@ export namespace AzureStorageClientUtility {
*/
export async function createDirectory(fileServerClient: azureStorage.FileService, azureFoler: any, azureShare: any): Promise<boolean> {
const deferred: Deferred<boolean> = new Deferred<boolean>();
fileServerClient.createDirectoryIfNotExists(azureShare, azureFoler, (error: any, result: any, response: any) => {
fileServerClient.createDirectoryIfNotExists(azureShare, azureFoler, (error: any, _result: any, _response: any) => {
if (error) {
getLogger()
.error(`Create directory failed:, ${error}`);
Expand Down Expand Up @@ -89,7 +89,7 @@ export namespace AzureStorageClientUtility {
localFilePath: string): Promise<boolean> {
const deferred: Deferred<boolean> = new Deferred<boolean>();
await fileServerClient.createFileFromLocalFile(azureShare, azureDirectory, azureFileName, localFilePath,
(error: any, result: any, response: any) => {
(error: any, _result: any, _response: any) => {
if (error) {
getLogger()
.error(`Upload file failed:, ${error}`);
Expand All @@ -114,7 +114,7 @@ export namespace AzureStorageClientUtility {
localFilePath: string): Promise<boolean> {
const deferred: Deferred<boolean> = new Deferred<boolean>();
await fileServerClient.getFileToStream(azureShare, azureDirectory, azureFileName, fs.createWriteStream(localFilePath),
(error: any, result: any, response: any) => {
(error: any, _result: any, _response: any) => {
if (error) {
getLogger()
.error(`Download file failed:, ${error}`);
Expand Down Expand Up @@ -183,7 +183,7 @@ export namespace AzureStorageClientUtility {
const deferred: Deferred<void> = new Deferred<void>();
await mkDirP(localDirectory);
fileServerClient.listFilesAndDirectoriesSegmented(azureShare, azureDirectory, 'null',
async (error: any, result: any, response: any) => {
async (_error: any, result: any, _response: any) => {
if (('entries' in result) === false) {
getLogger()
.error(`list files failed, can't get entries in result`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ export class KubernetesJobInfoCollector {
await Promise.all(updateKubernetesTrialJobs);
}

protected async retrieveSingleTrialJobInfo(kubernetesCRDClient: KubernetesCRDClient | undefined,
kubernetesTrialJob: KubernetesTrialJobDetail): Promise<void> {
protected async retrieveSingleTrialJobInfo(_kubernetesCRDClient: KubernetesCRDClient | undefined,
_kubernetesTrialJob: KubernetesTrialJobDetail): Promise<void> {
throw new MethodNotImplementedError();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ abstract class KubernetesTrainingService {
public async listTrialJobs(): Promise<TrialJobDetail[]> {
const jobs: TrialJobDetail[] = [];

for (const [key, value] of this.trialJobsMap) {
for (const key of this.trialJobsMap.keys()) {
jobs.push(await this.getTrialJob(key));
}

Expand Down Expand Up @@ -107,7 +107,7 @@ abstract class KubernetesTrainingService {
return false;
}

public getClusterMetadata(key: string): Promise<string> {
public getClusterMetadata(_key: string): Promise<string> {
return Promise.resolve('');
}

Expand Down
1 change: 0 additions & 1 deletion src/nni_manager/training_service/pai/paiConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

'use strict';

import {TrialConfig} from '../common/trialConfig';
import { TrialJobApplicationForm, TrialJobDetail, TrialJobStatus } from '../../common/trainingService';

export class PAIClusterConfig {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class PAIJobInfoCollector {
};

//TODO : pass in request timeout param?
request(getJobInfoRequest, (error: Error, response: request.Response, body: any) => {
request(getJobInfoRequest, (error: Error, response: request.Response, _body: any) => {
if ((error !== undefined && error !== null) || response.statusCode >= 500) {
this.log.error(`PAI Training service: get job info for trial ${paiTrialJob.id} from PAI Cluster failed!`);
// Queried PAI job info failed, set job status to UNKNOWN
Expand Down
2 changes: 0 additions & 2 deletions src/nni_manager/training_service/pai/paiJobRestServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
'use strict';

import { Request, Response, Router } from 'express';
import { Inject } from 'typescript-ioc';
import * as component from '../../common/component';
import { ClusterJobRestServer } from '../common/clusterJobRestServer';
import { PAITrainingService } from './paiTrainingService';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

'use strict';

import * as cpp from 'child-process-promise';
import * as fs from 'fs';
import * as path from 'path';
// tslint:disable-next-line:no-implicit-dependencies
Expand All @@ -29,11 +28,13 @@ import * as component from '../../../common/component';
import { Deferred } from 'ts-deferred';
import { String } from 'typescript-string-operations';
import {
HyperParameters, NNIManagerIpConfig, TrainingService,
TrialJobApplicationForm, TrialJobDetail, TrialJobMetric
HyperParameters, NNIManagerIpConfig,
TrialJobApplicationForm, TrialJobDetail
} from '../../../common/trainingService';
import { delay, generateParamFileName,
getExperimentRootDir, getIPV4Address, getVersion, uniqueString, unixPathJoin } from '../../../common/utils';
import {
generateParamFileName,
getIPV4Address, getVersion, uniqueString
} from '../../../common/utils';
import { CONTAINER_INSTALL_NNI_SHELL_FORMAT } from '../../common/containerJobData';
import { TrialConfigMetadataKey } from '../../common/trialConfigMetadataKey';
import { execMkdir, validateCodeDir, execCopydir } from '../../common/util';
Expand All @@ -56,7 +57,7 @@ class PAIK8STrainingService extends PAITrainingService {
private nniVersion: string | undefined;
constructor() {
super();

}

public async setClusterMetadata(key: string, value: string): Promise<void> {
Expand All @@ -69,10 +70,10 @@ class PAIK8STrainingService extends PAITrainingService {
this.paiJobRestServer = new PAIJobRestServer(component.get(PAIK8STrainingService));
this.paiClusterConfig = <PAIClusterConfig>JSON.parse(value);
this.paiClusterConfig.host = this.formatPAIHost(this.paiClusterConfig.host);
if(this.paiClusterConfig.passWord) {
if (this.paiClusterConfig.passWord) {
// Get PAI authentication token
await this.updatePaiToken();
} else if(this.paiClusterConfig.token) {
} else if (this.paiClusterConfig.token) {
this.paiToken = this.paiClusterConfig.token;
}
break;
Expand Down Expand Up @@ -104,7 +105,7 @@ class PAIK8STrainingService extends PAITrainingService {
this.log.error(`Uknown key: ${key}`);
}
}

// update trial parameters for multi-phase
public async updateTrialJob(trialJobId: string, form: TrialJobApplicationForm): Promise<TrialJobDetail> {
const trialJobDetail: PAITrialJobDetail | undefined = this.trialJobsMap.get(trialJobId);
Expand Down Expand Up @@ -146,8 +147,8 @@ class PAIK8STrainingService extends PAITrainingService {

return trialJobDetail;
}
private generateNNITrialCommand(trialJobDetail: PAITrialJobDetail, command: string) {

private generateNNITrialCommand(trialJobDetail: PAITrialJobDetail, command: string): string {
if (this.paiTrialConfig === undefined) {
throw new Error('trial config is not initialized');
}
Expand All @@ -167,13 +168,13 @@ class PAIK8STrainingService extends PAITrainingService {
this.nniVersion,
this.logCollection
)
.replace(/\r\n|\n|\r/gm, '');
.replace(/\r\n|\n|\r/gm, '');

return nniPaiTrialCommand;

}

private generateJobConfigInYamlFormat(trialJobDetail: PAITrialJobDetail) {
private generateJobConfigInYamlFormat(trialJobDetail: PAITrialJobDetail): any {
if (this.paiTrialConfig === undefined) {
throw new Error('trial config is not initialized');
}
Expand All @@ -185,31 +186,31 @@ class PAIK8STrainingService extends PAITrainingService {
nniJobConfig.name = jobName;
// Each taskRole will generate new command in NNI's command format
// Each command will be formatted to NNI style
for(const taskRoleIndex in nniJobConfig.taskRoles) {
for (const taskRoleIndex in nniJobConfig.taskRoles) {
const commands = nniJobConfig.taskRoles[taskRoleIndex].commands
const nniTrialCommand = this.generateNNITrialCommand(trialJobDetail, commands.join(" && ").replace(/(["'$`\\])/g,'\\$1'));
const nniTrialCommand = this.generateNNITrialCommand(trialJobDetail, commands.join(" && ").replace(/(["'$`\\])/g, '\\$1'));
nniJobConfig.taskRoles[taskRoleIndex].commands = [nniTrialCommand]
}

} else {
nniJobConfig = {
protocolVersion: 2,
protocolVersion: 2,
name: jobName,
type: 'job',
jobRetryCount: 0,
prerequisites: [
{
type: 'dockerimage',
uri: this.paiTrialConfig.image,
name: 'docker_image_0'
}
{
type: 'dockerimage',
uri: this.paiTrialConfig.image,
name: 'docker_image_0'
}
],
taskRoles: {
taskrole: {
instances: 1,
completion: {
minFailedInstances: 1,
minSucceededInstances: -1
minFailedInstances: 1,
minSucceededInstances: -1
},
taskRetryCount: 0,
dockerImage: 'docker_image_0',
Expand Down
Loading

0 comments on commit 6485ef1

Please sign in to comment.