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

feat: bulk docs prop #1568

Merged
merged 43 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
79b3f8d
refactor: simplify sync request handling in DocsEditViewPanel
AdiGajbhiye Jan 28, 2025
4055ae7
refactor: optimize loading logic in DocumentationPropagationButton
AdiGajbhiye Jan 28, 2025
4d86e95
feat: add Loader component and integrate into DocumentationPropagatio…
AdiGajbhiye Jan 28, 2025
8d9b624
feat: implement cancellation logic in documentation propagation and i…
AdiGajbhiye Jan 28, 2025
b76807b
refactor: streamline column handling in DocumentationPropagationButto…
AdiGajbhiye Jan 28, 2025
3b55605
feat: add bulk documentation propagation panel and update state manag…
AdiGajbhiye Jan 28, 2025
647d7ad
feat: enhance DocumentationPropagationButton with Card component for …
AdiGajbhiye Jan 28, 2025
d783a30
feat: refactor DocumentationPropagationButton to use SingleColumnCard…
AdiGajbhiye Jan 28, 2025
45c0d1d
feat: enhance BulkDocumentationPropagationPanel with loading state an…
AdiGajbhiye Jan 29, 2025
1428e23
refactor: simplify column handling in BulkDocumentationPropagationPan…
AdiGajbhiye Jan 29, 2025
b11fe97
refactor: extract logic into useDocumentationPropagation hook for bet…
AdiGajbhiye Jan 29, 2025
6bdc0f3
feat: implement mergeDocItems function and enhance useDocumentationPr…
AdiGajbhiye Jan 29, 2025
52896fd
feat: add Select All and Unselect All buttons for column selection in…
AdiGajbhiye Jan 29, 2025
e99d753
feat: add dispatch for closing drawer and canceling column lineage in…
AdiGajbhiye Jan 29, 2025
137c99b
refactor: streamline cancellation token handling in DbtLineageService…
AdiGajbhiye Jan 29, 2025
3e14b4f
feat: reorganize layout and styling for BulkDocumentationPropagationP…
AdiGajbhiye Feb 4, 2025
d6d2e38
feat: enhance BulkDocumentationPropagationPanel with sorting and impr…
AdiGajbhiye Feb 4, 2025
2ec0d7f
feat: conditionally render description in SingleColumnCard and adjust…
AdiGajbhiye Feb 4, 2025
e7947f2
feat: add loading state for column lineage in BulkDocumentationPropag…
AdiGajbhiye Feb 4, 2025
a39901d
feat: implement expandable rows in SingleColumnCard with loading stat…
AdiGajbhiye Feb 4, 2025
d9ea7d6
refactor: simplify state management by replacing reduce with Object.f…
AdiGajbhiye Feb 5, 2025
12dae29
refactor: remove unused cancellation token logging in NewLineagePanel
AdiGajbhiye Feb 5, 2025
ba57bd2
feat: add telemetry event for bulk documentation save action
AdiGajbhiye Feb 5, 2025
d0ae564
feat: enhance DocumentationPropagationButton with data-testid attribu…
AdiGajbhiye Feb 5, 2025
09ed21a
feat: add data-testid attributes to SingleColumnCard for improved tes…
AdiGajbhiye Feb 5, 2025
3def681
feat: update propagateDocumentation calls to close drawer after execu…
AdiGajbhiye Feb 6, 2025
f81ee4e
feat: enhance bulk documentation save with success message for saved …
AdiGajbhiye Feb 6, 2025
065e9eb
feat: add bulkDocsPropCredit method and telemetry for successful docu…
AdiGajbhiye Feb 7, 2025
79df631
feat: remove isSaved state and related UI elements from documentation…
AdiGajbhiye Feb 7, 2025
97cfa01
feat: remove isSaved state and success message from DocumentationProp…
AdiGajbhiye Feb 7, 2025
311ade3
feat: update bulk documentation save to include number of columns in …
AdiGajbhiye Feb 8, 2025
bbb5b4e
fixes
mdesmet Feb 9, 2025
b558082
feat: refactor setSelectedColumns to use functional update for improv…
AdiGajbhiye Feb 10, 2025
90d6b91
feat: remove SAVE_PROCEED action and related logic from Documentation…
AdiGajbhiye Feb 10, 2025
363c1e0
feat: integrate bulk documentation properties management in Documenta…
AdiGajbhiye Feb 10, 2025
6453333
feat: add support for single documentation properties panel in Docume…
AdiGajbhiye Feb 10, 2025
41e6fc7
feat: simplify warning message in DocumentationProvider for unsaved c…
AdiGajbhiye Feb 10, 2025
0edcce6
feat: enhance documentation propagation components with improved stat…
AdiGajbhiye Feb 10, 2025
baedd3f
feat: update documentation propagation logic to reset state and impro…
AdiGajbhiye Feb 10, 2025
3dc39c7
feat: streamline documentation saving process by removing unnecessary…
AdiGajbhiye Feb 10, 2025
502e26c
feat: reset tests and selected columns metadata in documentation prop…
AdiGajbhiye Feb 10, 2025
408f172
feat: remove unnecessary await from information message display in Do…
AdiGajbhiye Feb 10, 2025
defe7f9
Fix string
mdesmet Feb 10, 2025
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: 12 additions & 0 deletions src/altimate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,11 @@ interface FeedbackResponse {
ok: boolean;
}

interface BulkDocsPropRequest {
num_columns: number;
session_id: string;
}

interface AltimateConfig {
key: string;
instance: string;
Expand Down Expand Up @@ -954,6 +959,13 @@ export class AltimateRequest {
});
}

async bulkDocsPropCredit(req: BulkDocsPropRequest) {
return this.fetch<FeedbackResponse>("dbt/v4/bulk-docs-prop-credits", {
method: "POST",
body: JSON.stringify(req),
});
}

async getPreConfiguredNotebooks() {
return this.fetch<PreconfiguredNotebookItem[]>(
"notebook/preconfigured/list",
Expand Down
93 changes: 22 additions & 71 deletions src/services/dbtLineageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,66 +51,15 @@ export type Table = {
packageName?: string;
};

class DerivedCancellationTokenSource extends CancellationTokenSource {
constructor(linkedToken: CancellationToken) {
super();
linkedToken.onCancellationRequested(() => {
super.cancel();
});
}
}

@provideSingleton(DbtLineageService)
export class DbtLineageService {
private cllProgressResolve: () => void = () => {};

public constructor(
private altimateRequest: AltimateRequest,
protected telemetry: TelemetryService,
private dbtTerminal: DBTTerminal,
private queryManifestService: QueryManifestService,
) {}

async handleColumnLineage(
{ event }: { event: CllEvents },
onCancel: () => void,
) {
if (event === CllEvents.START) {
window.withProgress(
{
title: "Retrieving column level lineage",
location: ProgressLocation.Notification,
cancellable: true,
},
async (_, token) => {
await new Promise<void>((resolve) => {
this.cancellationTokenSource = new DerivedCancellationTokenSource(
token,
);
this.cllProgressResolve = resolve;
token.onCancellationRequested(() => {
onCancel();
});
});
},
);
return;
}
this.cancellationTokenSource?.token.onCancellationRequested((e) => {
console.log(e);
});
if (event === CllEvents.END) {
this.cllProgressResolve();
this.cancellationTokenSource?.dispose();
return;
}
if (event === CllEvents.CANCEL) {
this.cllProgressResolve();
this.cancellationTokenSource?.cancel();
return;
}
}

getUpstreamTables({ table }: { table: string }) {
return { tables: this.getConnectedTables("children", table) };
}
Expand Down Expand Up @@ -260,23 +209,25 @@ export class DbtLineageService {
return g.get(key)?.nodes.length || 0;
}

private cancellationTokenSource: CancellationTokenSource | undefined;
async getConnectedColumns({
targets,
upstreamExpansion,
currAnd1HopTables,
selectedColumn,
showIndirectEdges,
eventType,
}: {
targets: [string, string][];
upstreamExpansion: boolean;
currAnd1HopTables: string[];
// select_column is used for pricing not business logic
selectedColumn: { name: string; table: string };
showIndirectEdges: boolean;
eventType: string;
}) {
async getConnectedColumns(
{
targets,
upstreamExpansion,
currAnd1HopTables,
selectedColumn,
showIndirectEdges,
eventType,
}: {
targets: [string, string][];
upstreamExpansion: boolean;
currAnd1HopTables: string[];
// select_column is used for pricing not business logic
selectedColumn: { name: string; table: string };
showIndirectEdges: boolean;
eventType: string;
},
cancellationTokenSource: CancellationTokenSource,
) {
const _event = this.queryManifestService.getEventByCurrentProject();
if (!_event) {
return;
Expand Down Expand Up @@ -317,15 +268,15 @@ export class DbtLineageService {
await project.getNodesWithDBColumns(
event,
modelsToFetch,
this.cancellationTokenSource!.token,
cancellationTokenSource.token,
);

const selected_column = {
model_node: mappedNode[selectedColumn.table],
column: selectedColumn.name,
};

if (this.cancellationTokenSource?.token.isCancellationRequested) {
if (cancellationTokenSource.token.isCancellationRequested) {
return { column_lineage: [] };
}

Expand Down Expand Up @@ -411,7 +362,7 @@ export class DbtLineageService {

const modelDialect = project.getAdapterType();
try {
if (this.cancellationTokenSource?.token.isCancellationRequested) {
if (cancellationTokenSource.token.isCancellationRequested) {
return { column_lineage: [] };
}
const sessionId = `${env.sessionId}-${selectedColumn.table}-${selectedColumn.name}`;
Expand Down
1 change: 1 addition & 0 deletions src/telemetry/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export enum TelemetryEvents {
"DocumentationEditor/FeedbackClick" = "DocumentationEditor/FeedbackClick",
"DocumentationEditor/BulkGenerateTests" = "DocumentationEditor/BulkGenerateTests",
"DocumentationEditor/SaveClick" = "DocumentationEditor/SaveClick",
"DocumentationEditor/SaveBulk" = "DocumentationEditor/SaveBulk",
"DocumentationEditor/SaveError" = "DocumentationEditor/SaveError",
"DocumentationEditor/SaveNewFilePathSelect" = "DocumentationEditor/SaveNewFilePathSelect",
"DocumentationEditor/BulkGenerateAllClick" = "DocumentationEditor/BulkGenerateAllClick",
Expand Down
68 changes: 43 additions & 25 deletions src/webview_provider/docsEditPanel.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { existsSync, readFileSync, writeFileSync } from "fs";
import {
CancellationToken,
CancellationTokenSource,
ColorThemeKind,
Disposable,
ProgressLocation,
Expand All @@ -13,6 +14,7 @@ import {
WebviewViewResolveContext,
window,
workspace,
env,
} from "vscode";
import { DBTProjectContainer } from "../manifest/dbtProjectContainer";
import {
Expand Down Expand Up @@ -110,6 +112,7 @@ export class DocsEditViewPanel implements WebviewViewProvider {
private eventMap: Map<string, ManifestCacheProjectAddedEvent> = new Map();
private _disposables: Disposable[] = [];
private onMessageDisposable: Disposable | undefined;
private cancellationTokenSource: CancellationTokenSource | undefined;

public constructor(
private dbtProjectContainer: DBTProjectContainer,
Expand Down Expand Up @@ -682,15 +685,6 @@ export class DocsEditViewPanel implements WebviewViewProvider {
panel: this._panel,
});
break;
case "columnLineageBase": {
this.dbtLineageService.handleColumnLineage(params, () => {
this._panel?.webview.postMessage({
command: "columnLineage",
args: { event: CllEvents.CANCEL },
});
});
break;
}
case "getDownstreamColumns": {
const targets = params.targets as [string, string][];
const testsResult = await Promise.all(
Expand Down Expand Up @@ -720,7 +714,7 @@ export class DocsEditViewPanel implements WebviewViewProvider {
this.handleSyncRequestFromWebview(
syncRequestId,
() => ({ column_lineage: [], tables: [], tests }),
command,
"response",
);
return;
}
Expand All @@ -729,23 +723,29 @@ export class DocsEditViewPanel implements WebviewViewProvider {
name: params.column as string,
};
const currAnd1HopTables = [...tables, ...targets.map((t) => t[0])];
const columns = await this.dbtLineageService.getConnectedColumns({
targets,
currAnd1HopTables,
selectedColumn,
upstreamExpansion: true,
showIndirectEdges: false,
eventType: "documentation_propagation",
});
this.cancellationTokenSource = new CancellationTokenSource();
const columns = await this.dbtLineageService.getConnectedColumns(
{
targets,
currAnd1HopTables,
selectedColumn,
upstreamExpansion: true,
showIndirectEdges: false,
eventType: "documentation_propagation",
},
this.cancellationTokenSource!,
);
Comment on lines +726 to +737
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add disposal of cancellation token source.

While the cancellation token implementation is good, the token source should be disposed of after use to prevent memory leaks.

Apply this diff to add proper disposal:

 case "getDownstreamColumns": {
   // ... existing code ...
   this.cancellationTokenSource = new CancellationTokenSource();
   const columns = await this.dbtLineageService.getConnectedColumns(
     // ... existing code ...
   );
+  this.cancellationTokenSource.dispose();
+  this.cancellationTokenSource = undefined;
   this.handleSyncRequestFromWebview(
     // ... existing code ...
   );
   break;
 }
 case "cancelColumnLineage": {
   this.cancellationTokenSource?.cancel();
+  this.cancellationTokenSource?.dispose();
+  this.cancellationTokenSource = undefined;
   break;
 }

Also applies to: 745-748

this.handleSyncRequestFromWebview(
syncRequestId,
() => {
return { ...columns, tables: _tables, tests };
},
command,
() => ({ ...columns, tables: _tables, tests }),
"response",
);
break;
}
case "cancelColumnLineage": {
this.cancellationTokenSource?.cancel();
break;
}
case "saveDocumentation":
this.telemetry.sendTelemetryEvent(
TelemetryEvents["DocumentationEditor/SaveClick"],
Expand All @@ -761,20 +761,37 @@ export class DocsEditViewPanel implements WebviewViewProvider {
break;
case "saveDocumentationBulk": {
this.telemetry.sendTelemetryEvent(
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous implementation used window.withProgress to provide visual feedback during the bulk save operation. Removing this might affect user experience in cases where saving takes longer. Consider if a progress indicator is still needed.

TelemetryEvents["DocumentationEditor/SaveClick"],
TelemetryEvents["DocumentationEditor/SaveBulk"],
);
window.withProgress(
const successfulSaves = await window.withProgress(
{
title: "Saving documentation",
location: ProgressLocation.Notification,
cancellable: false,
},
async () => {
const successfulSaves: string[] = [];
for (const item of message.models) {
await this.saveDocumentation(item, syncRequestId);
const model = await this.saveDocumentation(
item,
syncRequestId,
);
if (model) {
successfulSaves.push(model);
}
}
return successfulSaves;
},
);
if (successfulSaves.length > 0) {
await window.showInformationMessage(
`Successfully saved: ${Array.from(new Set(successfulSaves)).join(", ")}`,
);
this.altimateRequest.bulkDocsPropCredit({
num_columns: message.numColumns,
session_id: env.sessionId,
});
}
break;
}
}
Expand Down Expand Up @@ -983,6 +1000,7 @@ export class DocsEditViewPanel implements WebviewViewProvider {
},
});
}
return this.documentation?.name;
} catch (error) {
this.transmitError();
this.telemetry.sendTelemetryError(
Expand Down
Loading
Loading