Use bricks sync in vscode extension (#105)

shreyas-goenka · web-flow · commit cbb8893bb0cb · 2022-09-21T18:21:41.000+02:00
Tested manually that 1. The sync works 2. Switching profiles is seamless wrt syncing https://user-images.githubusercontent.com/88374338/191528171-b4ab1901-2d16-4c39-b16f-0dcfbf56666c.mov Question: how are we planning to bundle the vscode extension with the bricks cli ?
diff --git a/packages/databricks-vscode/package.json b/packages/databricks-vscode/package.json
@@ -461,7 +461,7 @@
     "scripts": {
         "vscode:prepublish": "rm -rf out && yarn run package:compile && yarn run package:copy-webview-toolkit",
         "package": "vsce package",
-        "package:cli:fetch": "BRICKS_VERSION=v0.0.2 && bash ./scripts/fetch-bricks-cli.sh ${BRICKS_VERSION} ${BRICKS_ARCH:-}",
+        "package:cli:fetch": "BRICKS_VERSION=v0.0.3 && bash ./scripts/fetch-bricks-cli.sh ${BRICKS_VERSION} ${BRICKS_ARCH:-}",
         "package:cli:link": "rm -f ./bin/bricks && ln -s ../../../../bricks/bricks bin",
         "package:compile": "yarn run esbuild:base --minify",
         "package:copy-webview-toolkit": "cp ./node_modules/@vscode/webview-ui-toolkit/dist/toolkit.js ./out/toolkit.js",
diff --git a/packages/databricks-vscode/src/cli/BricksTasks.test.ts b/packages/databricks-vscode/src/cli/BricksTasks.test.ts
@@ -48,16 +48,9 @@ describe(__filename, () => {
         );
 
         let cliMock = mock(CliWrapper);
-        when(
-            cliMock.getSyncCommand(
-                anything(),
-                anything(),
-                anything(),
-                anything()
-            )
-        ).thenReturn({
+        when(cliMock.getSyncCommand(anything())).thenReturn({
             command: "bricks",
-            args: ["sync", "incremental"],
+            args: ["sync"],
         });
 
         let task = new SyncTask(
@@ -69,17 +62,12 @@ describe(__filename, () => {
 
         let execution = task.execution as ProcessExecution;
 
-        const syncCommandMock = cliMock.getSyncCommand(
-            anything(),
-            anything(),
-            anything(),
-            anything()
-        );
+        const syncCommandMock = cliMock.getSyncCommand(anything());
 
         verify(syncCommandMock).never();
         assert.equal(execution.process, "bricks");
         verify(syncCommandMock).once();
-        assert.deepEqual(execution.args, ["sync", "incremental"]);
+        assert.deepEqual(execution.args, ["sync"]);
         verify(syncCommandMock).once();
     });
 });
diff --git a/packages/databricks-vscode/src/cli/BricksTasks.ts b/packages/databricks-vscode/src/cli/BricksTasks.ts
@@ -30,6 +30,10 @@ export class SyncTask extends Task {
     constructor(
         connection: ConnectionManager,
         cli: CliWrapper,
+        // TODO: https://github.com/databricks/databricks-vscode/issues/111
+        // use syncType to decide the sync type for bricks cli. Right now bricks cli
+        // only supports full sync for multiple profiles.
+        // see: https://github.com/databricks/bricks/issues/71
         syncType: "full" | "incremental"
     ) {
         super(
@@ -88,8 +92,24 @@ class LazySyncProcessExecution extends ProcessExecution {
                         throw new Error("!!!!!");
                     }
 
+                    const profile = this.connection.profile;
+                    if (!profile) {
+                        window.showErrorMessage(
+                            "Can't start sync: Databricks connection not configured!"
+                        );
+                        throw new Error(
+                            "Can't start sync: Databricks connection not configured!"
+                        );
+                    }
+
                     return {
                         cwd: workspacePath,
+                        env: {
+                            /* eslint-disable @typescript-eslint/naming-convention */
+                            BRICKS_ROOT: workspacePath,
+                            DATABRICKS_CONFIG_PROFILE: profile,
+                            /* eslint-enable @typescript-eslint/naming-convention */
+                        },
                     };
                 },
             },
@@ -100,19 +120,8 @@ class LazySyncProcessExecution extends ProcessExecution {
         if (this.command) {
             return this.command;
         }
-
-        const me = this.connection.me;
         const syncDestination = this.connection.syncDestination;
-        const profile = this.connection.profile;
 
-        if (!me || !profile) {
-            window.showErrorMessage(
-                "Can't start sync: Databricks connection not configured!"
-            );
-            throw new Error(
-                "Can't start sync: Databricks connection not configured!"
-            );
-        }
         if (!syncDestination) {
             window.showErrorMessage(
                 "Can't start sync: Databricks synchronization destination not configured!"
@@ -122,12 +131,7 @@ class LazySyncProcessExecution extends ProcessExecution {
             );
         }
 
-        this.command = this.cli.getSyncCommand(
-            profile,
-            me,
-            syncDestination,
-            this.syncType
-        );
+        this.command = this.cli.getSyncCommand(syncDestination);
 
         return this.command;
     }
diff --git a/packages/databricks-vscode/src/cli/CliWrapper.test.ts b/packages/databricks-vscode/src/cli/CliWrapper.test.ts
@@ -29,43 +29,11 @@ describe(__filename, () => {
             Uri.file("/Users/fabian.jakobs/Desktop/notebook-best-practices")
         );
 
-        const {command, args} = cli.getSyncCommand(
-            "DEFAULT",
-            "fabian@databricks.com",
-            mapper,
-            "incremental"
-        );
-
-        assert.equal(
-            [command, ...args].join(" "),
-            "dbx sync repo --profile DEFAULT --user fabian@databricks.com --dest-repo notebook-best-practices"
-        );
-    });
-
-    it("should create full sync command", () => {
-        const cli = new CliWrapper({
-            asAbsolutePath(path: string) {
-                return path;
-            },
-        } as any);
-        const mapper = new SyncDestination(
-            Uri.from({
-                scheme: "dbws",
-                path: "/Workspace/Repos/fabian.jakobs@databricks.com/notebook-best-practices",
-            }),
-            Uri.file("/Users/fabian.jakobs/Desktop/notebook-best-practices")
-        );
-
-        const {command, args} = cli.getSyncCommand(
-            "DEFAULT",
-            "fabian@databricks.com",
-            mapper,
-            "full"
-        );
+        const {command, args} = cli.getSyncCommand(mapper);
 
         assert.equal(
             [command, ...args].join(" "),
-            "dbx sync repo --profile DEFAULT --user fabian@databricks.com --dest-repo notebook-best-practices --full-sync"
+            "./bin/bricks sync --remote-path /Repos/fabian.jakobs@databricks.com/notebook-best-practices --persist-snapshot=false"
         );
     });
 
diff --git a/packages/databricks-vscode/src/cli/CliWrapper.ts b/packages/databricks-vscode/src/cli/CliWrapper.ts
@@ -24,30 +24,16 @@ export class CliWrapper {
     }
 
     /**
-     * Constructs the dbx sync command
+     * Constructs the bricks sync command
      */
-    getSyncCommand(
-        profile: string,
-        me: string,
-        syncDestination: SyncDestination,
-        syncType: "full" | "incremental"
-    ): Command {
-        const command = "dbx";
+    getSyncCommand(syncDestination: SyncDestination): Command {
+        const command = this.context.asAbsolutePath("./bin/bricks");
         const args = [
             "sync",
-            "repo",
-            "--profile",
-            profile,
-            "--user",
-            me,
-            "--dest-repo",
-            syncDestination.name,
+            "--remote-path",
+            syncDestination.relativeRepoPath,
+            "--persist-snapshot=false",
         ];
-
-        if (syncType === "full") {
-            args.push("--full-sync");
-        }
-
         return {command, args};
     }
 
diff --git a/packages/databricks-vscode/src/cluster/ClusterManager.test.ts b/packages/databricks-vscode/src/cluster/ClusterManager.test.ts
@@ -33,6 +33,7 @@ describe(__filename, async () => {
                 "/api/2.0/clusters/get",
                 "GET",
                 deepEqual({
+                    // eslint-disable-next-line
                     cluster_id: testClusterDetails.cluster_id,
                 }),
                 anything()
@@ -59,6 +60,7 @@ describe(__filename, async () => {
                 "/api/2.0/clusters/get",
                 "GET",
                 deepEqual({
+                    // eslint-disable-next-line
                     cluster_id: testClusterDetails.cluster_id,
                 }),
                 anything()
diff --git a/packages/databricks-vscode/src/configuration/SyncDestination.ts b/packages/databricks-vscode/src/configuration/SyncDestination.ts
@@ -36,6 +36,10 @@ export class SyncDestination {
         return this.repoPath;
     }
 
+    get relativeRepoPath(): string {
+        return this.repoPath.path.replace("/Workspace", "");
+    }
+
     /**
      * Strips the workspace path from an absolute path
      */
