Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop downloading odo/oc when cluster is down #406

Merged
merged 6 commits into from
Nov 28, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
12 changes: 3 additions & 9 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,9 @@ export class Cli implements ICli {
childProcess.exec(cmd, opts, (error: ExecException, stdout: string, stderr: string) => {
this.odoChannel.print(stdout);
this.odoChannel.print(stderr);
if (error) {
if (error.code === 1 && stderr.trim().length === 0) {
resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr });
} else {
reject(error);
}
} else {
resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr });
}
// do not reject it here, because caller in some cases need the error and the streams
// to make a decision
resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr });
});
});
}
Expand Down
18 changes: 12 additions & 6 deletions src/odo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export interface Odo {
getServiceTemplatePlans(svc: string): Promise<string[]>;
getServices(application: OpenShiftObject): Promise<OpenShiftObject[]>;
getApplicationChildren(application: OpenShiftObject): Promise<OpenShiftObject[]>;
execute(command: string, cwd?: string): Promise<CliExitData>;
execute(command: string, cwd?: string, fail?: boolean): Promise<CliExitData>;
requireLogin(): Promise<boolean>;
}

Expand Down Expand Up @@ -207,9 +207,9 @@ export class OdoImpl implements Odo {
return clusters;
}

public async getClustersWithOc(): Promise<OpenShiftObject[]> {
private async getClustersWithOc(): Promise<OpenShiftObject[]> {
let clusters: OpenShiftObject[] = [];
const result: cliInstance.CliExitData = await this.execute(`oc version`);
const result: cliInstance.CliExitData = await this.execute(`oc version`, process.cwd(), false);
clusters = result.stdout.trim().split('\n').filter((value) => {
return value.indexOf('Server ') !== -1;
}).map((value) => {
Expand All @@ -219,7 +219,7 @@ export class OdoImpl implements Odo {
return clusters;
}

public async getClustersWithOdo(): Promise<OpenShiftObject[]> {
private async getClustersWithOdo(): Promise<OpenShiftObject[]> {
let clusters: OpenShiftObject[] = [];
const result: cliInstance.CliExitData = await this.execute(
`odo version && odo project list`
Expand Down Expand Up @@ -294,13 +294,19 @@ export class OdoImpl implements Odo {
terminal.show();
}

public async execute(command: string, cwd?: string): Promise<CliExitData> {
public async execute(command: string, cwd?: string, fail: boolean = true): Promise<CliExitData> {
const cmd = command.split(' ')[0];
const toolLocation = await ToolsConfig.detectOrDownload(cmd);
return OdoImpl.cli.execute(
toolLocation ? command.replace(cmd, `"${toolLocation}"`).replace(new RegExp(`&& ${cmd}`, 'g'), `&& "${toolLocation}"`) : command,
cwd ? {cwd} : { }
);
).then(async (result) => {
if (result.error && fail) {
return Promise.reject(result.error);
} else {
return result;
}
});
}

public async requireLogin(): Promise<boolean> {
Expand Down
18 changes: 7 additions & 11 deletions src/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,18 +162,14 @@ export class ToolsConfig {
let detectedVersion: string;
if (fs.existsSync(location)) {
const version = new RegExp(`${cmd} v([\\d\\.]+)`);
try {
const result = await Cli.getInstance().execute(`"${location}" version`);
if (!result.error) {
const toolVersion: string[] = result.stdout.trim().split('\n').filter((value) => {
return value.match(version);
}).map((value)=>version.exec(value)[1]);
if (toolVersion.length) {
detectedVersion = toolVersion[0];
}
const result = await Cli.getInstance().execute(`"${location}" version`);
if (result.stdout) {
const toolVersion: string[] = result.stdout.trim().split('\n').filter((value) => {
return value.match(version);
}).map((value)=>version.exec(value)[1]);
if (toolVersion.length) {
detectedVersion = toolVersion[0];
}
} catch (ignore) {
// if `${tool} version` failed, then there is no tool at specified location
}
}
return detectedVersion;
Expand Down
32 changes: 19 additions & 13 deletions src/util/progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import * as vscode from 'vscode';
import * as odoctl from '../odo';
import { CliExitData } from '../cli';
import { ExecException } from 'child_process';

export interface Step {
command: string;
Expand All @@ -20,30 +22,34 @@ export class Progress {
(progress: vscode.Progress<{increment: number, message: string}>, token: vscode.CancellationToken) => {
const calls: (()=>Promise<any>)[] = [];
steps.reduce((previous: Step, current: Step, currentIndex: number, steps: Step[])=> {
current.total = previous.total + current.increment;
calls.push(()=> {
let result: Promise<any> = Promise.resolve();
progress.report({
increment: previous.increment,
message: `${previous.total}%`
});
result = odo.execute(current.command);
current.total = previous.total + current.increment;
return currentIndex+1 === steps.length ? result.then(()=> {
return new Promise<CliExitData>(async (resolve,reject) => {
progress.report({
increment: previous.increment,
message: `${previous.total}%`
});
}) : result;

const result: CliExitData = await odo.execute(current.command);
Copy link
Contributor

Choose a reason for hiding this comment

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

If an error happens here, it gets swallowed and the progress is stalled. You might need to call this with fail = false to pass it into the handling below.

if(result.error) {
reject(result.error);
} else {
if (currentIndex+1 === steps.length) {
progress.report({
increment: current.increment,
message: `${current.total}%`
});
}
resolve();
}
});
});
return current;
}, {increment: 0, command: "", total: 0});

return calls.reduce<Promise<any>>((previous: Promise<any>, current: ()=>Promise<any>, index: number, calls: (()=>Promise<any>)[])=> {
return previous.then(current);
}, Promise.resolve()).catch((error) => {
vscode.window.showErrorMessage(`${error}`);
return;
});
}, Promise.resolve());
});
}
}
17 changes: 11 additions & 6 deletions test/util/progress.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ suite('Progress Utility', () => {

setup(() => {
sandbox = sinon.createSandbox();
execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves();
});

teardown(() => {
sandbox.restore();
});

test('calls cli commands in sequence', async () => {
execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves({ error: undefined, stdout: "", stderr: "" });
await Progress.execWithProgress(options, steps, OdoImpl.getInstance());

expect(execStub).calledTwice;
Expand All @@ -46,17 +46,22 @@ suite('Progress Utility', () => {
});

test('calls progress with given options', async () => {
execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves({ error: undefined, stdout: "", stderr: "" });
const spy = sandbox.spy(vscode.window, 'withProgress');
await Progress.execWithProgress(options, steps, OdoImpl.getInstance());

expect(spy).calledOnceWith(options, sinon.match.func);
});

test('shows an error message if a command fails', async () => {
execStub.rejects(errorMessage);
test('throw an error if a command fails', async () => {
let error = new Error(errorMessage);
execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves({ error, stdout: "", stderr: "" });
const spy = sandbox.spy(vscode.window, 'showErrorMessage');
await Progress.execWithProgress(options, steps, OdoImpl.getInstance());

expect(spy).calledOnceWith(errorMessage);
try {
await Progress.execWithProgress(options, steps, OdoImpl.getInstance());
expect.fail(false,true, 'no errors were detected');
} catch(err) {
expect(err).equals(error);
}
});
});