-
Notifications
You must be signed in to change notification settings - Fork 22
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use databricks CLI log-file option to capture the logs (#923)
## Changes The CLI can write its logs to a file and we don't have to manage this on the extension side. - The format of the logs is a bit different now - In the verbose mode we were writing debug logs to the terminal task in the VSCode (as they were coming to the stderr). Right now such logs are only in the log file, and output has just the stdout (as CLI keeps stderr empty when you ask it to log to a file) - The verbose mode option is gone. The output in the sync terminal is clean and always based on stdout, and the verbose logs are always written into a file ## Tests Updated existing tests, added new unit tests
- Loading branch information
Showing
9 changed files
with
109 additions
and
131 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,62 +5,100 @@ import { | |
| RemoteUri, | ||
| SyncDestinationMapper, | ||
| } from "../sync/SyncDestination"; | ||
| import {workspaceConfigs} from "../vscode-objs/WorkspaceConfigs"; | ||
| import {promisify} from "node:util"; | ||
| import {execFile as execFileCb} from "node:child_process"; | ||
| import {withFile} from "tmp-promise"; | ||
| import {writeFile} from "node:fs/promises"; | ||
|
|
||
| import {writeFile, readFile} from "node:fs/promises"; | ||
| import {when, spy, reset} from "ts-mockito"; | ||
| import {CliWrapper} from "./CliWrapper"; | ||
| import path from "node:path"; | ||
| import os from "node:os"; | ||
| import crypto from "node:crypto"; | ||
| import {Context} from "@databricks/databricks-sdk/dist/context"; | ||
| import {logging} from "@databricks/databricks-sdk"; | ||
|
|
||
| const execFile = promisify(execFileCb); | ||
| const cliPath = path.join(__dirname, "../../bin/databricks"); | ||
|
|
||
| function getTempLogFilePath() { | ||
| return path.join( | ||
| os.tmpdir(), | ||
| `databricks-cli-logs-${crypto.randomUUID()}.json` | ||
| ); | ||
| } | ||
|
|
||
| function createCliWrapper(logFilePath?: string) { | ||
| return new CliWrapper( | ||
| { | ||
| asAbsolutePath(relativePath: string) { | ||
| return path.join(__dirname, "../..", relativePath); | ||
| }, | ||
| } as any, | ||
| logFilePath | ||
| ); | ||
| } | ||
|
|
||
| describe(__filename, () => { | ||
| it("should embed a working databricks CLI", async () => { | ||
| const cliPath = __dirname + "/../../bin/databricks"; | ||
| const result = await execFile(cliPath, ["--help"]); | ||
| assert.ok(result.stdout.indexOf("databricks") > 0); | ||
| }); | ||
|
|
||
| let mocks: any[] = []; | ||
| afterEach(() => { | ||
| mocks.forEach((mock) => reset(mock)); | ||
| mocks = []; | ||
| }); | ||
|
|
||
| it("should tell CLI to log its output to a file", async () => { | ||
| const logFilePath = getTempLogFilePath(); | ||
| const configsSpy = spy(workspaceConfigs); | ||
| mocks.push(configsSpy); | ||
| when(configsSpy.loggingEnabled).thenReturn(true); | ||
| const cli = createCliWrapper(logFilePath); | ||
| await execFile(cli.cliPath, ["version", ...cli.loggingArguments]); | ||
| const file = await readFile(logFilePath); | ||
| // Just checking if the file is not empty to avoid depending on internal CLI log patterns | ||
| assert.ok(file.toString().length > 0); | ||
| }); | ||
|
|
||
| it("should create sync commands", async () => { | ||
| const cli = new CliWrapper({ | ||
| asAbsolutePath(path: string) { | ||
| return path; | ||
| }, | ||
| } as any); | ||
| const logFilePath = getTempLogFilePath(); | ||
| const cli = createCliWrapper(logFilePath); | ||
| const mapper = new SyncDestinationMapper( | ||
| new LocalUri( | ||
| Uri.file("/Users/fabian.jakobs/Desktop/notebook-best-practices") | ||
| ), | ||
| new LocalUri(Uri.file("/user/project")), | ||
| new RemoteUri( | ||
| Uri.from({ | ||
| scheme: "wsfs", | ||
| path: "/Repos/fabian.jakobs@databricks.com/notebook-best-practices", | ||
| path: "/Repos/user@databricks.com/project", | ||
| }) | ||
| ) | ||
| ); | ||
|
|
||
| const syncCommand = `${cliPath} sync . /Repos/[email protected]/project --watch --output json`; | ||
| const loggingArgs = `--log-level debug --log-file ${logFilePath} --log-format json`; | ||
| let {command, args} = cli.getSyncCommand(mapper, "incremental"); | ||
| assert.equal( | ||
| [command, ...args].join(" "), | ||
| "./bin/databricks sync . /Repos/[email protected]/notebook-best-practices --watch --output json" | ||
| [syncCommand, loggingArgs].join(" ") | ||
| ); | ||
|
|
||
| ({command, args} = cli.getSyncCommand(mapper, "full")); | ||
| assert.equal( | ||
| [command, ...args].join(" "), | ||
| "./bin/databricks sync . /Repos/[email protected]/notebook-best-practices --watch --output json --full" | ||
| [syncCommand, loggingArgs, "--full"].join(" ") | ||
| ); | ||
|
|
||
| const configsSpy = spy(workspaceConfigs); | ||
| mocks.push(configsSpy); | ||
| when(configsSpy.loggingEnabled).thenReturn(false); | ||
| ({command, args} = cli.getSyncCommand(mapper, "incremental")); | ||
| assert.equal([command, ...args].join(" "), syncCommand); | ||
| }); | ||
|
|
||
| it("should create an 'add profile' command", () => { | ||
| const cli = new CliWrapper({ | ||
| asAbsolutePath(path: string) { | ||
| return path; | ||
| }, | ||
| } as any); | ||
| const cli = createCliWrapper(); | ||
|
|
||
| const {command, args} = cli.getAddProfileCommand( | ||
| "DEFAULT", | ||
|
|
@@ -69,27 +107,20 @@ describe(__filename, () => { | |
|
|
||
| assert.equal( | ||
| [command, ...args].join(" "), | ||
| "./bin/databricks configure --no-interactive --profile DEFAULT --host https://databricks.com/ --token" | ||
| `${cliPath} configure --no-interactive --profile DEFAULT --host https://databricks.com/ --token` | ||
| ); | ||
| }); | ||
|
|
||
| it("should list profiles when no config file exists", async () => { | ||
| const cli = new CliWrapper({ | ||
| asAbsolutePath(p: string) { | ||
| return path.join(__dirname, "..", "..", p); | ||
| }, | ||
| } as any); | ||
|
|
||
| const logFilePath = getTempLogFilePath(); | ||
| const cli = createCliWrapper(logFilePath); | ||
| const profiles = await cli.listProfiles("/tmp/does-not-exist"); | ||
| assert.equal(profiles.length, 0); | ||
| }); | ||
|
|
||
| it("should list profiles", async function () { | ||
| const cli = new CliWrapper({ | ||
| asAbsolutePath(p: string) { | ||
| return path.join(__dirname, "..", "..", p); | ||
| }, | ||
| } as any); | ||
| const logFilePath = getTempLogFilePath(); | ||
| const cli = createCliWrapper(logFilePath); | ||
|
|
||
| await withFile(async ({path}) => { | ||
| await writeFile( | ||
|
|
@@ -121,11 +152,8 @@ describe(__filename, () => { | |
| }); | ||
|
|
||
| it("should load all valid profiles and return errors for rest", async () => { | ||
| const cli = new CliWrapper({ | ||
| asAbsolutePath(p: string) { | ||
| return path.join(__dirname, "..", "..", p); | ||
| }, | ||
| } as any); | ||
| const logFilePath = getTempLogFilePath(); | ||
| const cli = createCliWrapper(logFilePath); | ||
|
|
||
| await withFile(async ({path}) => { | ||
| await writeFile( | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.