Skip to content

Commit db34a5b

Browse files
kartikgupta-dbfjakobs
kartikgupta-db
and
fjakobs
authored
[DECO-208] Don't try to start sync if 1 task is already running (#147)
- If a sync task was already running, "run on databricks" would try to start another sync task, leading to error prompt. This PR fixes it. - Also add a new error when sync is stopped before finishing when executing a file. ## Before this change https://user-images.githubusercontent.com/88345179/198888420-7e26de66-af25-40e4-a40c-49cd5614cbae.mov ## After the change Uploading Screen Recording 2022-10-30 at 21.22.02.mov… Co-authored-by: Fabian Jakobs <fabian.jakobs@databricks.com>
1 parent 9f2a163 commit db34a5b

10 files changed

Lines changed: 363 additions & 313 deletions

File tree

‎packages/databricks-vscode/src/cli/BricksSyncParser.test.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import assert from "assert";
2+
import {mock} from "ts-mockito";
3+
import {EventEmitter} from "vscode";
4+
import {SyncState} from "../sync";
5+
import {BricksSyncParser} from "./BricksSyncParser";
6+
7+
describe("tests for BricksSycnParser", () => {
8+
let syncState: SyncState = "STOPPED";
9+
let bricksSycnParser: BricksSyncParser;
10+
11+
const syncStateCallback = (state: SyncState) => {
12+
syncState = state;
13+
};
14+
15+
beforeEach(() => {
16+
syncState = "STOPPED";
17+
bricksSycnParser = new BricksSyncParser(
18+
syncStateCallback,
19+
mock(EventEmitter<string>)
20+
);
21+
});
22+
23+
it("processing empty logs transitions sync status from STOPPED -> IN_PROGRESS and we wait for initial sync complete", () => {
24+
assert.equal(syncState, "STOPPED");
25+
bricksSycnParser.process("");
26+
assert.equal(syncState, "IN_PROGRESS");
27+
bricksSycnParser.process("[INFO] Initial Sync Complete");
28+
assert.equal(syncState, "WATCHING_FOR_CHANGES");
29+
});
30+
31+
it("processing action log transitions sync status from STOPPED -> INPROGRESS", () => {
32+
assert.equal(syncState, "STOPPED");
33+
bricksSycnParser.process("Action: PUT: hello.txt");
34+
assert.equal(syncState, "IN_PROGRESS");
35+
});
36+
37+
it("test bricksSycnParser.process correctly keeps track of state of inflight requests", () => {
38+
// recieving some random logs from bricks sync
39+
assert.equal(syncState, "STOPPED");
40+
bricksSycnParser.process("some random logs");
41+
assert.equal(syncState, "IN_PROGRESS");
42+
43+
// upload hello.txt
44+
bricksSycnParser.process("Action: PUT: hello.txt");
45+
assert.equal(syncState, "IN_PROGRESS");
46+
bricksSycnParser.process("Uploaded hello.txt");
47+
assert.equal(syncState, "IN_PROGRESS");
48+
bricksSycnParser.process("[INFO] Initial Sync Complete");
49+
assert.equal(syncState, "WATCHING_FOR_CHANGES");
50+
51+
// delete bye.txt
52+
bricksSycnParser.process("Action: DELETE: bye.txt");
53+
assert.equal(syncState, "IN_PROGRESS");
54+
bricksSycnParser.process("Deleted bye.txt");
55+
assert.equal(syncState, "WATCHING_FOR_CHANGES");
56+
57+
// both upload and delete some random prefix string that should be ignored
58+
bricksSycnParser.process(
59+
"[INFO] foo bar Action: PUT: a.txt DELETE: b.txt"
60+
);
61+
bricksSycnParser.process("Uploaded a.txt");
62+
assert.equal(syncState, "IN_PROGRESS");
63+
bricksSycnParser.process("Deleted b.txt");
64+
assert.equal(syncState, "WATCHING_FOR_CHANGES");
65+
66+
// upload and delete multiple files
67+
bricksSycnParser.process(
68+
"Action: PUT: a.txt, c.txt DELETE: b.txt, d.txt"
69+
);
70+
bricksSycnParser.process("Uploaded a.txt");
71+
assert.equal(syncState, "IN_PROGRESS");
72+
bricksSycnParser.process("Deleted b.txt");
73+
assert.equal(syncState, "IN_PROGRESS");
74+
bricksSycnParser.process("Deleted d.txt");
75+
assert.equal(syncState, "IN_PROGRESS");
76+
bricksSycnParser.process("Uploaded c.txt");
77+
assert.equal(syncState, "WATCHING_FOR_CHANGES");
78+
79+
// multi line logs
80+
bricksSycnParser.process(
81+
"Action: PUT: a.txt, c.txt DELETE: b.txt, d.txt\n" +
82+
"Uploaded a.txt\n" +
83+
"some random text\n" +
84+
"Uploaded c.txt"
85+
);
86+
bricksSycnParser.process("Deleted b.txt");
87+
assert.equal(syncState, "IN_PROGRESS");
88+
bricksSycnParser.process("Deleted d.txt");
89+
assert.equal(syncState, "WATCHING_FOR_CHANGES");
90+
});
91+
92+
it("uploaded logs for untracked files throw errors", () => {
93+
assert.throws(
94+
() => {
95+
bricksSycnParser.process("Uploaded a.txt");
96+
},
97+
{
98+
message: /untracked file uploaded/,
99+
}
100+
);
101+
});
102+
103+
it("delete logs for untracked files throw errors", () => {
104+
assert.throws(
105+
() => {
106+
bricksSycnParser.process("Deleted a.txt");
107+
},
108+
{
109+
message: /untracked file deleted/,
110+
}
111+
);
112+
});
113+
});

‎packages/databricks-vscode/src/cli/BricksSyncParser.ts‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import {EventEmitter} from "vscode";
2+
import {SyncState} from "../sync";
3+
4+
export class BricksSyncParser {
5+
private filesBeingUploaded = new Set<string>();
6+
private filesBeingDeleted = new Set<string>();
7+
private firstSyncDone = false;
8+
9+
constructor(
10+
private syncStateCallback: (state: SyncState) => void,
11+
private writeEmitter: EventEmitter<string>
12+
) {}
13+
14+
// Assumes we recieve a single line of bricks logs
15+
// A value bricks action looks like this
16+
// const s1 = "Action: PUT: g, .gitignore, DELETE: f"
17+
// A hacky way to solve this, lets move to structed logs from bricks later
18+
private parseForActionsInitiated(line: string) {
19+
var indexOfAction = line.indexOf("Action:");
20+
// The log line is not relevant for actions
21+
if (indexOfAction === -1) {
22+
return;
23+
}
24+
25+
const tokenizedLine = line.substring(indexOfAction).split(" ");
26+
var isPut = false;
27+
var isDelete = false;
28+
for (let i = 1; i < tokenizedLine.length; i++) {
29+
switch (tokenizedLine[i]) {
30+
case "PUT:": {
31+
isPut = true;
32+
isDelete = false;
33+
break;
34+
}
35+
case "DELETE:": {
36+
isDelete = true;
37+
isPut = false;
38+
break;
39+
}
40+
default: {
41+
// trim the trailing , if it exists
42+
var filePath = tokenizedLine[i].replace(/,$/, "");
43+
if (isPut) {
44+
this.filesBeingUploaded.add(filePath);
45+
} else if (isDelete) {
46+
this.filesBeingDeleted.add(filePath);
47+
} else {
48+
throw new Error(
49+
"[BricksSyncParser] unexpected logs recieved"
50+
);
51+
}
52+
}
53+
}
54+
}
55+
}
56+
57+
// We expect a single line of logs for all files being put/delete
58+
private parseForUploadCompleted(line: string) {
59+
var indexOfUploaded = line.indexOf("Uploaded");
60+
if (indexOfUploaded === -1) {
61+
return;
62+
}
63+
64+
const tokenizedLine = line.substring(indexOfUploaded).split(" ");
65+
if (tokenizedLine.length !== 2) {
66+
throw new Error("[BricksSyncParser] unexpected logs recieved");
67+
}
68+
const filePath = tokenizedLine[1];
69+
if (!this.filesBeingUploaded.has(filePath)) {
70+
throw new Error(
71+
"[BricksSyncParser] untracked file uploaded. All upload complete " +
72+
"logs should be preceded with a uploaded initialted log. file: " +
73+
filePath +
74+
". log recieved: `" +
75+
line +
76+
"`"
77+
);
78+
}
79+
this.filesBeingUploaded.delete(filePath);
80+
}
81+
82+
private parseForDeleteCompleted(line: string) {
83+
var indexOfDeleted = line.indexOf("Deleted");
84+
if (indexOfDeleted === -1) {
85+
return;
86+
}
87+
88+
const tokenizedLine = line.substring(indexOfDeleted).split(" ");
89+
if (tokenizedLine.length !== 2) {
90+
throw new Error("[BricksSyncParser] unexpected logs recieved");
91+
}
92+
const filePath = tokenizedLine[1];
93+
if (!this.filesBeingDeleted.has(filePath)) {
94+
throw new Error(
95+
"[BricksSyncParser] untracked file deleted. All delete complete " +
96+
"logs should be preceded with a delete initialted log. file: " +
97+
filePath +
98+
". log recieved: `" +
99+
line +
100+
"`"
101+
);
102+
}
103+
this.filesBeingDeleted.delete(filePath);
104+
}
105+
106+
// We block on execing any commands on vscode until we get a message from
107+
// bricks cli that the initial sync is done
108+
private parseForFirstSync(line: string) {
109+
var indexOfSyncComplete = line.indexOf("Initial Sync Complete");
110+
if (indexOfSyncComplete !== -1) {
111+
this.firstSyncDone = true;
112+
}
113+
}
114+
115+
// This function processes the stderr logs from bricks sync and parses it
116+
// to compute the sync state ie determine whether the remote files match
117+
// what we have stored locally.
118+
// TODO: Use structed logging to compute the sync state here
119+
public process(data: string) {
120+
var logLines = data.split("\n");
121+
for (let i = 0; i < logLines.length; i++) {
122+
var line = logLines[i];
123+
this.parseForActionsInitiated(line);
124+
this.parseForUploadCompleted(line);
125+
this.parseForDeleteCompleted(line);
126+
127+
if (!this.firstSyncDone) {
128+
this.parseForFirstSync(line);
129+
}
130+
131+
// this.writeEmitter.fire writes to the pseudoterminal for the
132+
// bricks sync process
133+
this.writeEmitter.fire(line.trim());
134+
135+
// When vscode flush prints the logs from events fired here,
136+
// it automatically adds a new line. Since we can reasonably expect
137+
// with a high probablity that all logs in one call of this process(data) func will
138+
// be flushed together, we do not add a new line at the last event
139+
// to keep the new line spacing consistant
140+
if (i !== logLines.length - 1) {
141+
this.writeEmitter.fire("\n\r");
142+
}
143+
}
144+
if (
145+
this.filesBeingDeleted.size === 0 &&
146+
this.filesBeingUploaded.size === 0 &&
147+
this.firstSyncDone
148+
) {
149+
this.syncStateCallback("WATCHING_FOR_CHANGES");
150+
} else {
151+
this.syncStateCallback("IN_PROGRESS");
152+
}
153+
}
154+
}

‎packages/databricks-vscode/src/cli/BricksTasks.test.ts‎

Lines changed: 1 addition & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {anything, instance, mock, when, verify} from "ts-mockito";
44
import {ProcessExecution, Uri, EventEmitter} from "vscode";
55
import {ConnectionManager} from "../configuration/ConnectionManager";
66
import {SyncState} from "../sync/CodeSynchronizer";
7-
import {BricksTaskProvider, SyncTask, BricksSyncParser} from "./BricksTasks";
7+
import {BricksTaskProvider, SyncTask} from "./BricksTasks";
88
import {CliWrapper} from "./CliWrapper";
99

1010
describe(__filename, () => {
@@ -39,111 +39,3 @@ describe(__filename, () => {
3939
assert.deepEqual(task.problemMatchers, ["$bricks-sync"]);
4040
});
4141
});
42-
43-
describe("tests for BricksSycnParser", () => {
44-
let syncState: SyncState = "STOPPED";
45-
let bricksSycnParser: BricksSyncParser;
46-
47-
const syncStateCallback = (state: SyncState) => {
48-
syncState = state;
49-
};
50-
51-
beforeEach(() => {
52-
syncState = "STOPPED";
53-
bricksSycnParser = new BricksSyncParser(
54-
syncStateCallback,
55-
mock(EventEmitter<string>)
56-
);
57-
});
58-
59-
it("processing empty logs transitions sync status from STOPPED -> IN_PROGRESS and we wait for initial sync complete", () => {
60-
assert.equal(syncState, "STOPPED");
61-
bricksSycnParser.process("");
62-
assert.equal(syncState, "IN_PROGRESS");
63-
bricksSycnParser.process("[INFO] Initial Sync Complete");
64-
assert.equal(syncState, "WATCHING_FOR_CHANGES");
65-
});
66-
67-
it("processing action log transitions sync status from STOPPED -> INPROGRESS", () => {
68-
assert.equal(syncState, "STOPPED");
69-
bricksSycnParser.process("Action: PUT: hello.txt");
70-
assert.equal(syncState, "IN_PROGRESS");
71-
});
72-
73-
it("test bricksSycnParser.process correctly keeps track of state of inflight requests", () => {
74-
// recieving some random logs from bricks sync
75-
assert.equal(syncState, "STOPPED");
76-
bricksSycnParser.process("some random logs");
77-
assert.equal(syncState, "IN_PROGRESS");
78-
79-
// upload hello.txt
80-
bricksSycnParser.process("Action: PUT: hello.txt");
81-
assert.equal(syncState, "IN_PROGRESS");
82-
bricksSycnParser.process("Uploaded hello.txt");
83-
assert.equal(syncState, "IN_PROGRESS");
84-
bricksSycnParser.process("[INFO] Initial Sync Complete");
85-
assert.equal(syncState, "WATCHING_FOR_CHANGES");
86-
87-
// delete bye.txt
88-
bricksSycnParser.process("Action: DELETE: bye.txt");
89-
assert.equal(syncState, "IN_PROGRESS");
90-
bricksSycnParser.process("Deleted bye.txt");
91-
assert.equal(syncState, "WATCHING_FOR_CHANGES");
92-
93-
// both upload and delete some random prefix string that should be ignored
94-
bricksSycnParser.process(
95-
"[INFO] foo bar Action: PUT: a.txt DELETE: b.txt"
96-
);
97-
bricksSycnParser.process("Uploaded a.txt");
98-
assert.equal(syncState, "IN_PROGRESS");
99-
bricksSycnParser.process("Deleted b.txt");
100-
assert.equal(syncState, "WATCHING_FOR_CHANGES");
101-
102-
// upload and delete multiple files
103-
bricksSycnParser.process(
104-
"Action: PUT: a.txt, c.txt DELETE: b.txt, d.txt"
105-
);
106-
bricksSycnParser.process("Uploaded a.txt");
107-
assert.equal(syncState, "IN_PROGRESS");
108-
bricksSycnParser.process("Deleted b.txt");
109-
assert.equal(syncState, "IN_PROGRESS");
110-
bricksSycnParser.process("Deleted d.txt");
111-
assert.equal(syncState, "IN_PROGRESS");
112-
bricksSycnParser.process("Uploaded c.txt");
113-
assert.equal(syncState, "WATCHING_FOR_CHANGES");
114-
115-
// multi line logs
116-
bricksSycnParser.process(
117-
"Action: PUT: a.txt, c.txt DELETE: b.txt, d.txt\n" +
118-
"Uploaded a.txt\n" +
119-
"some random text\n" +
120-
"Uploaded c.txt"
121-
);
122-
bricksSycnParser.process("Deleted b.txt");
123-
assert.equal(syncState, "IN_PROGRESS");
124-
bricksSycnParser.process("Deleted d.txt");
125-
assert.equal(syncState, "WATCHING_FOR_CHANGES");
126-
});
127-
128-
it("uploaded logs for untracked files throw errors", () => {
129-
assert.throws(
130-
() => {
131-
bricksSycnParser.process("Uploaded a.txt");
132-
},
133-
{
134-
message: /untracked file uploaded/,
135-
}
136-
);
137-
});
138-
139-
it("delete logs for untracked files throw errors", () => {
140-
assert.throws(
141-
() => {
142-
bricksSycnParser.process("Deleted a.txt");
143-
},
144-
{
145-
message: /untracked file deleted/,
146-
}
147-
);
148-
});
149-
});

0 commit comments

Comments
 (0)