Skip to content

Commit

Permalink
Allow CLI path to have spaces (#1295)
Browse files Browse the repository at this point in the history
## Changes
<!-- Summary of your changes that are easy to understand -->

## Tests
<!-- How is this tested? -->
  • Loading branch information
kartikgupta-db authored Aug 13, 2024
1 parent c96b05f commit 5dc0308
Show file tree
Hide file tree
Showing 11 changed files with 189 additions and 124 deletions.
5 changes: 3 additions & 2 deletions packages/databricks-vscode/src/bundle/BundleInitWizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {Events, Telemetry} from "../telemetry";
import {OverrideableConfigModel} from "../configuration/models/OverrideableConfigModel";
import {writeFile, mkdir} from "fs/promises";
import path from "path";
import {escapePathArgument} from "../utils/shellUtils";

export async function promptToOpenSubProjects(
projects: {absolute: Uri; relative: string}[],
Expand Down Expand Up @@ -188,12 +189,12 @@ export class BundleInitWizard {
"bundle",
"init",
"--output-dir",
this.cli.escapePathArgument(parentFolder.fsPath),
escapePathArgument(parentFolder.fsPath),
].join(" ");
const initialPrompt = `clear; echo "Executing: databricks ${args}\nFollow the steps below to create your new Databricks project.\n"`;
const finalPrompt = `echo "\nPress any key to close the terminal and continue ..."; ${ShellUtils.readCmd()}; exit`;
terminal.sendText(
`${initialPrompt}; ${this.cli.cliPath} ${args}; ${finalPrompt}`
`${initialPrompt}; ${this.cli.escapedCliPath} ${args}; ${finalPrompt}`
);
return new Promise<void>((resolve) => {
const closeEvent = window.onDidCloseTerminal(async (t) => {
Expand Down
1 change: 0 additions & 1 deletion packages/databricks-vscode/src/cli/CliWrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ nothing = true
PATH: process.env.PATH,
/* eslint-enable @typescript-eslint/naming-convention */
}),
shell: true,
},
};
try {
Expand Down
55 changes: 46 additions & 9 deletions packages/databricks-vscode/src/cli/CliWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,42 @@ import {quote} from "shell-quote";
import {BundleVariableModel} from "../bundle/models/BundleVariableModel";
import {MsPythonExtensionWrapper} from "../language/MsPythonExtensionWrapper";
import path from "path";
import {isPowershell} from "../utils/shellUtils";

const withLogContext = logging.withLogContext;
const execFile = promisify(execFileCb);
function getEscapedCommandAndAgrs(
cmd: string,
args: string[],
options: SpawnOptionsWithoutStdio
) {
if (process.platform === "win32") {
args = [
"/d", //Disables execution of AutoRun commands, which are like .bashrc commands.
"/c", //Carries out the command specified by <string> and then exits the command processor.
`""${cmd}" ${args.map((a) => `"${a}"`).join(" ")}"`,
];
cmd = "cmd.exe";
options = {...options, windowsVerbatimArguments: true};
}
return {cmd, args, options};
}

export const execFile = async (
file: string,
args: string[],
options: any = {}
): Promise<{
stdout: string;
stderr: string;
}> => {
const {
cmd,
args: escapedArgs,
options: escapedOptions,
} = getEscapedCommandAndAgrs(file, args, options);
const res = await promisify(execFileCb)(cmd, escapedArgs, escapedOptions);
return {stdout: res.stdout.toString(), stderr: res.stderr.toString()};
};

export interface Command {
command: string;
Expand Down Expand Up @@ -154,13 +187,14 @@ async function runBundleCommand(
});

logger?.debug(quote([cmd, ...args]), {bundleOpName});
let options: SpawnOptionsWithoutStdio = {
cwd: workspaceFolder.fsPath,
env: removeUndefinedKeys(env),
};

({cmd, args, options} = getEscapedCommandAndAgrs(cmd, args, options));
try {
const p = spawn(cmd, args, {
cwd: workspaceFolder.fsPath,
env: removeUndefinedKeys(env),
shell: true,
});
const p = spawn(cmd, args, options);

const {stdout, stderr} = await waitForProcess(p, onStdOut, onStdError);
logger?.info(displayLogs.end, {
Expand Down Expand Up @@ -236,8 +270,10 @@ export class CliWrapper {
};
}

escapePathArgument(arg: string): string {
return `"${arg.replaceAll('"', '\\"')}"`;
get escapedCliPath(): string {
return isPowershell()
? `& "${this.cliPath.replace('"', '\\"')}"`
: `'${this.cliPath.replaceAll("'", "\\'")}'`;
}

/**
Expand Down Expand Up @@ -300,6 +336,8 @@ export class CliWrapper {
"Failed to parse Databricks Config File, please make sure it's in the correct ini format";
} else if (e.message.includes("spawn UNKNOWN")) {
msg = `Failed to parse Databricks Config File using databricks CLI, please make sure you have permissions to execute this binary: "${this.cliPath}"`;
} else {
msg += e.message;
}
}
ctx?.logger?.error(msg, e);
Expand Down Expand Up @@ -593,7 +631,6 @@ export class CliWrapper {
options: {
cwd: workspaceFolder.fsPath,
env,
shell: true,
},
};
}
Expand Down
38 changes: 25 additions & 13 deletions packages/databricks-vscode/src/configuration/LoginWizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
ValidationMessageType,
} from "../ui/MultiStepInputWizard";
import {CliWrapper, ConfigEntry} from "../cli/CliWrapper";
import {workspaceConfigs} from "../vscode-objs/WorkspaceConfigs";
import {
AuthProvider,
AuthType,
Expand All @@ -31,6 +30,8 @@ import ini from "ini";
import {appendFile, copyFile} from "fs/promises";
import path from "path";
import os from "os";
import {createFile} from "fs-extra";
import {getDatabricksConfigFilePath} from "../utils/fileUtils";

interface AuthTypeQuickPickItem extends QuickPickItem {
authType?: SdkAuthType;
Expand Down Expand Up @@ -384,25 +385,36 @@ export async function saveNewProfile(
throw new Error("Can't save empty auth provider to a profile");
}

const {path: configFilePath} = await loadConfigFile(
workspaceConfigs.databrickscfgLocation
);
const configFilePath: string = getDatabricksConfigFilePath().fsPath;
let shouldBackup = true;
try {
await loadConfigFile(configFilePath);
} catch (e) {
shouldBackup = false;
await createFile(configFilePath);
window.showInformationMessage(
`Created a new .databrickscfg file at ${configFilePath}`
);
}

const profile: any = {};
profile[profileName] = Object.fromEntries(
Object.entries(iniData).filter((kv) => kv[1] !== undefined)
);
const iniStr = ini.stringify(profile);
const finalStr = `${os.EOL};This profile is autogenerated by the Databricks Extension for VS Code${os.EOL}${iniStr}`;
// Create a backup for .databrickscfg
const backup = path.join(
path.dirname(configFilePath),
`.databrickscfg.${Date.now()}.bak`
);
await copyFile(configFilePath, backup);
window.showInformationMessage(
`Created a backup for .databrickscfg at ${backup}`
);

if (shouldBackup) {
// Create a backup for .databrickscfg
const backup = path.join(
path.dirname(configFilePath),
`.databrickscfg.${Date.now()}.bak`
);
await copyFile(configFilePath, backup);
window.showInformationMessage(
`Created a backup for .databrickscfg at ${backup}`
);
}

// Write the new profile to .databrickscfg
await appendFile(configFilePath, finalStr);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
ExecUtils,
ProductVersion,
WorkspaceClient,
logging,
Expand All @@ -8,6 +7,7 @@ import {Disposable, window} from "vscode";
import {DatabricksCliAuthProvider} from "./AuthProvider";
import {orchestrate, OrchestrationLoopError, Step} from "./orchestrate";
import {Loggers} from "../../logger";
import {execFile} from "../../cli/CliWrapper";

// eslint-disable-next-line @typescript-eslint/no-var-requires
const extensionVersion = require("../../../package.json")
Expand Down Expand Up @@ -92,7 +92,7 @@ export class DatabricksCliCheck implements Disposable {

private async login(): Promise<void> {
try {
await ExecUtils.execFile(this.authProvider.cliPath, [
await execFile(this.authProvider.cliPath, [
"auth",
"login",
"--host",
Expand Down
3 changes: 3 additions & 0 deletions packages/databricks-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,9 @@ export async function activate(
bundleRemoteStateModel,
configModel,
connectionManager,
commands.registerCommand("databricks.internal.showOutput", () => {
loggerManager.showOutputChannel("Databricks Logs");
}),
connectionManager.onDidChangeState(async () => {
telemetry.setMetadata(
Metadata.USER,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ import {StateStorage} from "../vscode-objs/StateStorage";
import {IExtensionApi as MsPythonExtensionApi} from "./MsPythonExtensionApi";
import {Mutex} from "../locking";
import * as childProcess from "node:child_process";
import {promisify} from "node:util";
import {WorkspaceFolderManager} from "../vscode-objs/WorkspaceFolderManager";
export const execFile = promisify(childProcess.execFile);
import {execFile} from "../cli/CliWrapper";

export class MsPythonExtensionWrapper implements Disposable {
public readonly api: MsPythonExtensionApi;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,12 @@ import {Loggers} from "../../logger";
import {Context, context} from "@databricks/databricks-sdk/dist/context";
import {Mutex} from "../../locking";
import {MsPythonExtensionWrapper} from "../MsPythonExtensionWrapper";
import {execFile as ef} from "child_process";
import {promisify} from "util";
import {FileUtils} from "../../utils";
import {workspaceConfigs} from "../../vscode-objs/WorkspaceConfigs";
import {LocalUri} from "../../sync/SyncDestination";
import {DatabricksEnvFileManager} from "../../file-managers/DatabricksEnvFileManager";
import {WorkspaceFolderManager} from "../../vscode-objs/WorkspaceFolderManager";

const execFile = promisify(ef);
import {execFile} from "../../cli/CliWrapper";

async function isDbnbTextEditor(editor?: TextEditor) {
try {
Expand Down
3 changes: 3 additions & 0 deletions packages/databricks-vscode/src/test/e2e/bundle_init.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,16 @@ describe("Bundle Init", async function () {
expect(await pick.getLabel()).toBe(parentDir);
await pick.select();

await dismissNotifications();

// Wait for the databricks cli terminal window to pop up and select all
// default options for the default template
const editorView = workbench.getEditorView();
const title = "Databricks Project Init";
const initTab = await getTabByTitle(title);
assert(initTab, "Can't find a tab for project-init terminal wizard");
await initTab.select();
await sleep(1000);

//select temaplate type
await browser.keys("default-python".split(""));
Expand Down
Loading

0 comments on commit 5dc0308

Please sign in to comment.