Skip to content

Commit 9f1c975

Browse files
committed
GH-1908 Preserve source database description when creating shared Glue databases
- Add optional description parameter to GlueClient.create_database() and _create_glue_database() - Retrieve and preserve source database description in LFShareManager - Update check_if_exists_and_create_shared_database_in_target() to pass source description - Add comprehensive test coverage for various description scenarios - Maintain backward compatibility with fallback to default description Fixes #1908
1 parent 509ebf1 commit 9f1c975

File tree

3 files changed

+93
-7
lines changed

3 files changed

+93
-7
lines changed

backend/dataall/modules/s3_datasets_shares/aws/glue_client.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,28 @@ def __init__(self, account_id, region, database):
1515
self._account_id = account_id
1616
self._region = region
1717

18-
def create_database(self, location):
18+
def create_database(self, location, description=None):
1919
try:
2020
log.info(f'Creating database {self._database} in account {self._account_id}...')
2121
existing_database = self.get_glue_database()
2222
if existing_database:
2323
glue_database_created = True
2424
else:
25-
self._create_glue_database(location)
25+
self._create_glue_database(location, description=description)
2626
glue_database_created = True
2727
log.info(f'Successfully created database {self._database} in account {self._account_id}')
2828
return glue_database_created
2929
except ClientError as e:
3030
log.error(f'Failed to create database {self._database} in account {self._account_id} due to {e}')
3131
raise e
3232

33-
def _create_glue_database(self, location):
33+
def _create_glue_database(self, location, description=None):
3434
database = self._database
3535
try:
36+
db_description = description or f'dataall database {database}'
3637
db_input = {
3738
'Name': database,
38-
'Description': 'dataall database {} '.format(database),
39+
'Description': db_description,
3940
'CreateTableDefaultPermissions': [],
4041
}
4142
if location:

backend/dataall/modules/s3_datasets_shares/services/share_managers/lf_share_manager.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,19 @@ def check_if_exists_and_create_shared_database_in_target(self) -> dict:
266266
"""
267267
Checks if shared database exists in target account
268268
Creates the shared database if it does not exist
269+
Preserves source database description if available
269270
:return: boto3 glue create_database
270271
"""
272+
source_database = self.glue_client_in_source.get_glue_database()
273+
source_description = None
274+
if source_database:
275+
source_description = source_database.get('Database', {}).get('Description')
271276

272-
database = self.glue_client_in_target.create_database(location=f's3://{self.dataset.S3BucketName}')
277+
# Create database in target with source description
278+
database = self.glue_client_in_target.create_database(
279+
location=f's3://{self.dataset.S3BucketName}',
280+
description=source_description,
281+
)
273282
return database
274283

275284
def grant_pivot_role_all_database_permissions_to_shared_database(self) -> True:

tests/modules/s3_datasets_shares/tasks/test_lf_share_manager.py

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,14 +298,90 @@ def test_grant_pivot_role_all_database_permissions_to_source_database(
298298
)
299299

300300

301-
def test_check_if_exists_and_create_shared_database_in_target(manager_with_mocked_clients, dataset1: S3Dataset):
301+
def test_check_if_exists_and_create_shared_database_in_target_with_source_description(
302+
manager_with_mocked_clients, dataset1: S3Dataset
303+
):
304+
"""Test that source database description is retrieved and passed to create_database"""
305+
manager, lf_client, glue_client, mock_glue_client = manager_with_mocked_clients
306+
glue_client.create_database.return_value = True
307+
glue_client.get_glue_database.return_value = {
308+
'Database': {
309+
'Name': dataset1.GlueDatabaseName,
310+
'Description': 'Production customer data - PII encrypted',
311+
}
312+
}
313+
# When
314+
manager.check_if_exists_and_create_shared_database_in_target()
315+
# Then
316+
glue_client.get_glue_database.assert_called_once()
317+
glue_client.create_database.assert_called_once()
318+
glue_client.create_database.assert_called_with(
319+
location=f's3://{dataset1.S3BucketName}',
320+
description='Production customer data - PII encrypted',
321+
)
322+
323+
324+
def test_check_if_exists_and_create_shared_database_in_target_without_source_description(
325+
manager_with_mocked_clients, dataset1: S3Dataset
326+
):
327+
"""Test that None description is passed when source database has no description"""
328+
manager, lf_client, glue_client, mock_glue_client = manager_with_mocked_clients
329+
glue_client.create_database.return_value = True
330+
glue_client.get_glue_database.return_value = {
331+
'Database': {
332+
'Name': dataset1.GlueDatabaseName,
333+
}
334+
}
335+
# When
336+
manager.check_if_exists_and_create_shared_database_in_target()
337+
# Then
338+
glue_client.get_glue_database.assert_called_once()
339+
glue_client.create_database.assert_called_once()
340+
glue_client.create_database.assert_called_with(
341+
location=f's3://{dataset1.S3BucketName}',
342+
description=None,
343+
)
344+
345+
346+
def test_check_if_exists_and_create_shared_database_in_target_source_db_not_found(
347+
manager_with_mocked_clients, dataset1: S3Dataset
348+
):
349+
"""Test that None description is passed when source database doesn't exist"""
350+
manager, lf_client, glue_client, mock_glue_client = manager_with_mocked_clients
351+
glue_client.create_database.return_value = True
352+
glue_client.get_glue_database.return_value = False
353+
# When
354+
manager.check_if_exists_and_create_shared_database_in_target()
355+
# Then
356+
glue_client.get_glue_database.assert_called_once()
357+
glue_client.create_database.assert_called_once()
358+
glue_client.create_database.assert_called_with(
359+
location=f's3://{dataset1.S3BucketName}',
360+
description=None,
361+
)
362+
363+
364+
def test_check_if_exists_and_create_shared_database_in_target_empty_description(
365+
manager_with_mocked_clients, dataset1: S3Dataset
366+
):
367+
"""Test that empty string description is passed through (fallback handled in GlueClient)"""
302368
manager, lf_client, glue_client, mock_glue_client = manager_with_mocked_clients
303369
glue_client.create_database.return_value = True
370+
glue_client.get_glue_database.return_value = {
371+
'Database': {
372+
'Name': dataset1.GlueDatabaseName,
373+
'Description': '',
374+
}
375+
}
304376
# When
305377
manager.check_if_exists_and_create_shared_database_in_target()
306378
# Then
379+
glue_client.get_glue_database.assert_called_once()
307380
glue_client.create_database.assert_called_once()
308-
glue_client.create_database.assert_called_with(location=f's3://{dataset1.S3BucketName}')
381+
glue_client.create_database.assert_called_with(
382+
location=f's3://{dataset1.S3BucketName}',
383+
description='',
384+
)
309385

310386

311387
def test_grant_pivot_role_all_database_permissions_to_shared_database(

0 commit comments

Comments
 (0)