Unity catalog explorer#1871
Conversation
| ) { | ||
| this.disposables.push( | ||
| this.connectionManager.onDidChangeState(() => { | ||
| this.catalogsCache = undefined; |
There was a problem hiding this comment.
do we need to clean the childrenCache here as well?
| return cached; | ||
| } | ||
| const result = await loader(); | ||
| this.childrenCache.set(key, result); |
There was a problem hiding this comment.
A transient API blip turns this into a sticky error state that survives until the user manually refreshes that exact node. Detect error/empty results and skip caching, or attach a TTL here.
There was a problem hiding this comment.
Done, skipping caching results with error nodes
| schemaName: string | ||
| ): Promise<UnityCatalogTreeNode[]> { | ||
| const [tablesResult, volumesResult, functionsResult, modelsResult] = | ||
| await Promise.allSettled([ |
There was a problem hiding this comment.
For workspaces with thousands of tables in a schema this is a heavy initial load, and loadSchemaChildren (loaders.ts:107) fires four such drains in parallel before the user sees anything. Consider adding a max-items cap with a "load more" sentinel
| "databricks.unityCatalog.showDetail", | ||
| showDetail | ||
| ), | ||
| treeView.onDidChangeSelection(async (event) => { |
There was a problem hiding this comment.
Performance concern for big catalogs:
Selection auto-opens the detail panel on every keyboard arrow — onDidChangeSelection fires on every focus change, not just clicks. Arrow-key browsing through a large tree will repeatedly create + post to the webview, and on schema/table selection will kick off loadNodeEnrichments (4 parallel API calls each). Consider gating this on an explicit user action or introduce a debounce.
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const stored: StoredFavoriteNode = {...node} as StoredFavoriteNode; |
There was a problem hiding this comment.
This will carry stale metadata as it deep-copies the whole node and stores it in workspace state.
Owner/comment/format/storageLocation will silently age until the table is re-fetched; renamed or deleted favorites will display indefinitely. Store only {kind, fullName, version?} and rehydrate from a fresh fetch when the Favorites root is expanded.
There was a problem hiding this comment.
I haven't found API for loading UC resources in bulk by fullnames, meaning if we rehydrate from a fresh fetch it will be N calls (where N is the number of pinned/favorite nodes). this might not be an issue, assuming most users might have 2-5 favorite resources
| }>; | ||
| } | ||
|
|
||
| async function loadChildrenForNode( |
There was a problem hiding this comment.
Does this re-implement loaders.ts:loadSchemaChildren for the no-cache path? Consider deduplicating; otherwise the two implementations will drift
There was a problem hiding this comment.
cleaned up detailLoader.ts, re-uses loaders, extracting mapping code into mappers.ts
| const staleKeys = new Set( | ||
| refs.filter((_, i) => results[i] === null).map(favoriteKey) | ||
| ); | ||
| if (staleKeys.size > 0) { | ||
| const cleaned = refs.filter((r) => !staleKeys.has(favoriteKey(r))); | ||
| await this.stateStorage.set( | ||
| "databricks.unityCatalog.favorites", | ||
| cleaned | ||
| ); | ||
| } |
There was a problem hiding this comment.
New issue introduced by the favorites-ref refactor: this prunes any ref whose loadFavoriteNode returned null and persists the removal. But loadFavoriteNode wraps everything in catch { return null }, so a transient network/5xx/auth error is indistinguishable from a genuinely deleted object — meaning a single API blip while expanding Favorites will permanently delete the user's favorites from workspace state.
This is the same failure mode as the error-caching comment from the last round (transient blip → wrong durable state), but worse here since it's destructive and persisted rather than just a stale cache entry.
Suggestion: only prune on a confirmed "not found" (404). Have loadFavoriteNode return a discriminated result, e.g. {status: "ok", node} | {status: "gone"} | {status: "error"}, and remove the ref only on "gone". On "error", keep the ref and surface an error/placeholder node instead of silently dropping it.
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Changes
Unity Catalog Features
Tree View
filterOnType)Node Types
Context Menu Actions
catalog.schema.table) for all nodes; copies just column name for columnsToolbar
Error Handling
Tests
UnityCatalogTreeDataProvider.test.ts