Skip to content

Commit 94bd3fd

Browse files
liuzulinsjc25-test
andauthored
fix(smus): Do not call getDatabases when a catalog is an empty parent catalog (#8363)
## Problem A bug is reported that when s3 table catalog is empty, there is getDatabases call being made and cause unwanted error ## Solution Handle the issue by not calling getDatabases when a catalog node is an empty parent node --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: Zulin Liu <zulil@amazon.com>
1 parent 17a0a98 commit 94bd3fd

File tree

5 files changed

+168
-5
lines changed

5 files changed

+168
-5
lines changed

packages/core/src/sagemakerunifiedstudio/explorer/nodes/lakehouseStrategy.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import {
3030
createColumnTreeItem,
3131
getColumnType,
3232
createErrorItem,
33+
isRedLakeCatalog,
34+
isS3TablesCatalog,
3335
} from './utils'
3436
import { createPlaceholderItem } from '../../../shared/treeview/utils'
3537
import { Column, Database, Table } from '@aws-sdk/client-glue'
@@ -353,7 +355,15 @@ async function getCatalogs(
353355
}
354356

355357
// For catalogs without children, create regular catalog node
356-
return createCatalogNode(parentCatalog.CatalogId || '', parentCatalog, glueClient, parent, false)
358+
// For RedLake and S3TableCatalog, they are supposed to have children
359+
// Pass isParent as true
360+
return createCatalogNode(
361+
parentCatalog.CatalogId || '',
362+
parentCatalog,
363+
glueClient,
364+
parent,
365+
isRedLakeCatalog(parentCatalog) || isS3TablesCatalog(parentCatalog)
366+
)
357367
})
358368
}
359369

@@ -385,7 +395,7 @@ function createCatalogNode(
385395
},
386396
// Child catalogs load databases, parent catalogs will have their children provider overridden
387397
isParent
388-
? async () => [] // Placeholder, will be overridden for parent catalogs with children
398+
? async () => [createPlaceholderItem(NO_DATA_FOUND_MESSAGE) as LakehouseNode]
389399
: async (node) => {
390400
try {
391401
logger.info(`Loading databases for catalog ${catalogId}`)

packages/core/src/sagemakerunifiedstudio/explorer/nodes/utils.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,31 @@ import { getContext } from '../../../shared/vscode/setContext'
2424
import { SmusAuthenticationProvider } from '../../auth/providers/smusAuthenticationProvider'
2525
import { SmusIamConnection } from '../../auth/model'
2626
import { ConnectionStatus } from '@aws-sdk/client-datazone'
27+
import { GlueCatalog } from '../../shared/client/glueCatalogClient'
2728

2829
/**
2930
* Polling interval in milliseconds for checking space status updates
3031
*/
3132
// eslint-disable-next-line @typescript-eslint/naming-convention
3233
export const PENDING_NODE_POLLING_INTERVAL_MS = 5000
3334

35+
/**
36+
* Check if a catalog is a RedLake catalog
37+
*/
38+
export const isRedLakeCatalog = (catalog?: GlueCatalog) => {
39+
return (
40+
catalog?.FederatedCatalog?.ConnectionName === 'aws:redshift' ||
41+
catalog?.CatalogProperties?.DataLakeAccessProperties?.CatalogType === 'aws:redshift'
42+
)
43+
}
44+
45+
/**
46+
* Check if a catalog is a S3 table catalog
47+
*/
48+
export const isS3TablesCatalog = (catalog?: GlueCatalog) => {
49+
return catalog?.FederatedCatalog?.ConnectionName === 'aws:s3tables'
50+
}
51+
3452
/**
3553
* Gets the label for a node based on its data
3654
*/

packages/core/src/test/sagemakerunifiedstudio/explorer/nodes/lakehouseStrategy.test.ts

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,16 @@ describe('LakehouseStrategy', function () {
220220
})
221221

222222
describe('Catalog nodes', function () {
223-
it('should create catalog nodes from API', async function () {
223+
it('should create regular catalog nodes and load databases', async function () {
224+
const regularCatalog = {
225+
CatalogId: 'regular-catalog',
226+
Name: 'regular-catalog',
227+
CatalogType: 'HIVE',
228+
}
229+
224230
mockGlueCatalogClient.getCatalogs.resolves({
225-
catalogs: [{ CatalogId: 'test-catalog', CatalogType: 'HIVE' }],
231+
catalogs: [regularCatalog],
232+
nextToken: undefined,
226233
})
227234
mockGlueClient.getDatabases.resolves({
228235
databases: [{ Name: 'test-db' }],
@@ -236,8 +243,13 @@ describe('LakehouseStrategy', function () {
236243
)
237244
const children = await node.getChildren()
238245

239-
assert.ok(children.length > 0)
246+
const catalogNode = children.find((child) => child.id === 'regular-catalog') as LakehouseNode
247+
assert.ok(catalogNode)
240248
assert.ok(mockGlueCatalogClient.getCatalogs.called)
249+
250+
const catalogChildren = await catalogNode.getChildren()
251+
assert.strictEqual(catalogChildren.length, 1)
252+
assert.strictEqual((catalogChildren[0] as LakehouseNode).data.nodeType, NodeType.GLUE_DATABASE)
241253
})
242254

243255
it('should handle catalog database pagination', async function () {
@@ -277,6 +289,64 @@ describe('LakehouseStrategy', function () {
277289
assert.strictEqual(children.length, 2)
278290
assert.ok(mockGlueClient.getDatabases.calledTwice)
279291
})
292+
293+
it('should treat RedLake catalogs as parent catalogs', async function () {
294+
const redLakeCatalog = {
295+
CatalogId: 'redlake-catalog',
296+
Name: 'redlake-catalog',
297+
FederatedCatalog: {
298+
ConnectionName: 'aws:redshift',
299+
},
300+
}
301+
302+
mockGlueCatalogClient.getCatalogs.resolves({
303+
catalogs: [redLakeCatalog],
304+
nextToken: undefined,
305+
})
306+
307+
const node = createLakehouseConnectionNode(
308+
mockConnection as any,
309+
mockCredentialsProvider as ConnectionCredentialsProvider,
310+
'us-east-1'
311+
)
312+
const children = await node.getChildren()
313+
314+
const catalogNode = children.find((child) => child.id === 'redlake-catalog') as LakehouseNode
315+
assert.ok(catalogNode)
316+
317+
const catalogChildren = await catalogNode.getChildren()
318+
assert.strictEqual(catalogChildren.length, 1)
319+
assert.ok(catalogChildren[0].resource === '[No data found]')
320+
})
321+
322+
it('should treat S3Tables catalogs as parent catalogs', async function () {
323+
const s3TablesCatalog = {
324+
CatalogId: 's3tables-catalog',
325+
Name: 's3tables-catalog',
326+
FederatedCatalog: {
327+
ConnectionName: 'aws:s3tables',
328+
},
329+
}
330+
331+
mockGlueCatalogClient.getCatalogs.resolves({
332+
catalogs: [s3TablesCatalog],
333+
nextToken: undefined,
334+
})
335+
336+
const node = createLakehouseConnectionNode(
337+
mockConnection as any,
338+
mockCredentialsProvider as ConnectionCredentialsProvider,
339+
'us-east-1'
340+
)
341+
const children = await node.getChildren()
342+
343+
const catalogNode = children.find((child) => child.id === 's3tables-catalog') as LakehouseNode
344+
assert.ok(catalogNode)
345+
346+
const catalogChildren = await catalogNode.getChildren()
347+
assert.strictEqual(catalogChildren.length, 1)
348+
assert.ok(catalogChildren[0].resource === '[No data found]')
349+
})
280350
})
281351

282352
describe('Database nodes', function () {

packages/core/src/test/sagemakerunifiedstudio/explorer/nodes/utils.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
isRedLakeDatabase,
1616
getTooltip,
1717
getRedshiftTypeFromHost,
18+
isRedLakeCatalog,
19+
isS3TablesCatalog,
1820
} from '../../../../sagemakerunifiedstudio/explorer/nodes/utils'
1921
import { NodeType, ConnectionType, RedshiftType } from '../../../../sagemakerunifiedstudio/explorer/nodes/types'
2022

@@ -249,4 +251,63 @@ describe('utils', function () {
249251
assert.strictEqual(getRedshiftTypeFromHost(unknownHost), undefined)
250252
})
251253
})
254+
255+
describe('isRedLakeCatalog', function () {
256+
it('should return true for RedLake catalogs with FederatedCatalog connection', function () {
257+
const catalog = {
258+
FederatedCatalog: {
259+
ConnectionName: 'aws:redshift',
260+
},
261+
}
262+
assert.strictEqual(isRedLakeCatalog(catalog), true)
263+
})
264+
265+
it('should return true for RedLake catalogs with CatalogProperties', function () {
266+
const catalog = {
267+
CatalogProperties: {
268+
DataLakeAccessProperties: {
269+
CatalogType: 'aws:redshift',
270+
},
271+
},
272+
}
273+
assert.strictEqual(isRedLakeCatalog(catalog), true)
274+
})
275+
276+
it('should return false for non-RedLake catalogs', function () {
277+
const catalog = {
278+
FederatedCatalog: {
279+
ConnectionName: 'aws:s3tables',
280+
},
281+
}
282+
assert.strictEqual(isRedLakeCatalog(catalog), false)
283+
})
284+
285+
it('should return false for undefined catalog', function () {
286+
assert.strictEqual(isRedLakeCatalog(undefined), false)
287+
})
288+
})
289+
290+
describe('isS3TablesCatalog', function () {
291+
it('should return true for S3 Tables catalogs', function () {
292+
const catalog = {
293+
FederatedCatalog: {
294+
ConnectionName: 'aws:s3tables',
295+
},
296+
}
297+
assert.strictEqual(isS3TablesCatalog(catalog), true)
298+
})
299+
300+
it('should return false for non-S3 Tables catalogs', function () {
301+
const catalog = {
302+
FederatedCatalog: {
303+
ConnectionName: 'aws:redshift',
304+
},
305+
}
306+
assert.strictEqual(isS3TablesCatalog(catalog), false)
307+
})
308+
309+
it('should return false for undefined catalog', function () {
310+
assert.strictEqual(isS3TablesCatalog(undefined), false)
311+
})
312+
})
252313
})
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "SageMaker Unified Studio: Fixed s3 table catalog node showing error when it's empty"
4+
}

0 commit comments

Comments
 (0)