Skip to content

Commit d21ca67

Browse files
committed
refactor: rename FeatureGroup to FeatureGroupManager
Rename the mlops FeatureGroup class to FeatureGroupManager to distinguish it from the core FeatureGroup base class. Update all references in unit and integration lake formation tests. Fix missing comma in __init__.py __all__ list. --- X-AI-Prompt: rename FeatureGroup to FeatureGroupManager and update lakeformation tests X-AI-Tool: kiro-cli
1 parent a6f4065 commit d21ca67

File tree

4 files changed

+247
-230
lines changed

4 files changed

+247
-230
lines changed

sagemaker-mlops/src/sagemaker/mlops/feature_store/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
"""SageMaker FeatureStore V3 - powered by sagemaker-core."""
44

55
# FeatureGroup with Lake Formation support (local subclass)
6-
from sagemaker.mlops.feature_store.feature_group import FeatureGroup, LakeFormationConfig
6+
from sagemaker.core.resources import FeatureGroup, FeatureMetadata
7+
from sagemaker.mlops.feature_store.feature_group_manager import FeatureGroupManager, LakeFormationConfig
78

89
# Resources from core
910
from sagemaker.core.resources import FeatureMetadata
@@ -76,6 +77,7 @@
7677
__all__ = [
7778
# Resources
7879
"FeatureGroup",
80+
"FeatureGroupManager",
7981
"FeatureMetadata",
8082
# Shapes
8183
"DataCatalogConfig",

sagemaker-mlops/src/sagemaker/mlops/feature_store/feature_group.py renamed to sagemaker-mlops/src/sagemaker/mlops/feature_store/feature_group_manager.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import botocore.exceptions
1010

11-
from sagemaker.core.resources import FeatureGroup as CoreFeatureGroup
11+
from sagemaker.core.resources import FeatureGroup
1212
from sagemaker.core.resources import Base
1313
from sagemaker.core.shapes import (
1414
FeatureDefinition,
@@ -55,11 +55,11 @@ class LakeFormationConfig(Base):
5555
disable_hybrid_access_mode: bool = True
5656

5757

58-
class FeatureGroup(CoreFeatureGroup):
58+
class FeatureGroupManager(FeatureGroup):
5959

6060
# Inherit parent docstring and append our additions
61-
if CoreFeatureGroup.__doc__ and __doc__:
62-
__doc__ = CoreFeatureGroup.__doc__
61+
if FeatureGroup.__doc__ and __doc__:
62+
__doc__ = FeatureGroup.__doc__
6363

6464
@staticmethod
6565
def _s3_uri_to_arn(s3_uri: str, region: Optional[str] = None) -> str:
@@ -326,7 +326,7 @@ def _revoke_iam_allowed_principal(
326326
},
327327
Permissions=["ALL"],
328328
)
329-
logger.info(f"Revoked IAMAllowedPrincipal from table: {database_name}.{table_name}")
329+
logger.info(f"Disabled Lake Formation hybrid-access mode on table: {database_name}.{table_name}")
330330
return True
331331
except botocore.exceptions.ClientError as e:
332332
# if the Table doesn't have that permission because the user already revoked it
@@ -514,6 +514,21 @@ def enable_lake_formation(
514514
table_name_str = str(table_name)
515515
role_arn_str = str(self.role_arn)
516516

517+
# Determine the actual S3 location to register with Lake Formation.
518+
# For Iceberg tables, the Glue table's StorageDescriptor.Location is the parent
519+
# path (without /data suffix), while resolved_output_s3_uri always ends with /data.
520+
s3_location_to_register = resolved_s3_uri_str
521+
if (
522+
self.offline_store_config.table_format is not None
523+
and str(self.offline_store_config.table_format) == "Iceberg"
524+
and resolved_s3_uri_str.endswith("/data")
525+
):
526+
s3_location_to_register = resolved_s3_uri_str[: -len("/data")]
527+
logger.info(
528+
f"Iceberg table format detected. Using parent S3 path for LF registration: "
529+
f"{s3_location_to_register}"
530+
)
531+
517532
# Execute Lake Formation setup with fail-fast behavior
518533
results = {
519534
"s3_registration": False,
@@ -524,7 +539,7 @@ def enable_lake_formation(
524539
# Phase 1: Register S3 with Lake Formation
525540
try:
526541
results["s3_registration"] = self._register_s3_with_lake_formation(
527-
resolved_s3_uri_str,
542+
s3_location_to_register,
528543
session,
529544
region,
530545
use_service_linked_role=use_service_linked_role,

sagemaker-mlops/tests/integ/test_featureStore_lakeformation.py

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
"""
2-
Integration tests for Lake Formation with FeatureGroup.
2+
Integration tests for Lake Formation with FeatureGroupManager.
33
44
These tests require:
55
- AWS credentials with Lake Formation and SageMaker permissions
@@ -18,7 +18,7 @@
1818

1919
from sagemaker.core.helper.session_helper import Session, get_execution_role
2020
from sagemaker.mlops.feature_store import (
21-
FeatureGroup,
21+
FeatureGroupManager,
2222
LakeFormationConfig,
2323
OfflineStoreConfig,
2424
OnlineStoreConfig,
@@ -58,7 +58,7 @@ def region():
5858
@pytest.fixture(scope="module")
5959
def shared_feature_group_for_negative_tests(s3_uri, role, region):
6060
"""
61-
Create a single FeatureGroup for negative tests that only need to verify
61+
Create a single FeatureGroupManager for negative tests that only need to verify
6262
error conditions without modifying the resource.
6363
6464
This fixture is module-scoped to be created once and shared across tests,
@@ -81,12 +81,12 @@ def generate_feature_group_name():
8181
return f"test-lf-fg-{uuid.uuid4().hex[:8]}"
8282

8383

84-
def create_test_feature_group(name: str, s3_uri: str, role_arn: str, region: str) -> FeatureGroup:
85-
"""Create a FeatureGroup with offline store for testing."""
84+
def create_test_feature_group(name: str, s3_uri: str, role_arn: str, region: str) -> FeatureGroupManager:
85+
"""Create a FeatureGroupManager with offline store for testing."""
8686

8787
offline_store_config = OfflineStoreConfig(s3_storage_config=S3StorageConfig(s3_uri=s3_uri))
8888

89-
fg = FeatureGroup.create(
89+
fg = FeatureGroupManager.create(
9090
feature_group_name=name,
9191
record_identifier_feature_name="record_id",
9292
event_time_feature_name="event_time",
@@ -99,12 +99,12 @@ def create_test_feature_group(name: str, s3_uri: str, role_arn: str, region: str
9999
return fg
100100

101101

102-
def cleanup_feature_group(fg: FeatureGroup):
102+
def cleanup_feature_group(fg: FeatureGroupManager):
103103
"""
104-
Delete a FeatureGroup and its associated Glue table.
104+
Delete a FeatureGroupManager and its associated Glue table.
105105
106106
Args:
107-
fg: The FeatureGroup to delete.
107+
fg: The FeatureGroupManager to delete.
108108
"""
109109
try:
110110
# Delete the Glue table if it exists
@@ -128,7 +128,7 @@ def cleanup_feature_group(fg: FeatureGroup):
128128
# Don't fail cleanup if Glue table deletion fails
129129
pass
130130

131-
# Delete the FeatureGroup
131+
# Delete the FeatureGroupManager
132132
fg.delete()
133133
except ClientError:
134134
# Don't fail cleanup if Glue table deletion fails
@@ -139,20 +139,20 @@ def cleanup_feature_group(fg: FeatureGroup):
139139
@pytest.mark.slow_test
140140
def test_create_feature_group_and_enable_lake_formation(s3_uri, role, region):
141141
"""
142-
Test creating a FeatureGroup and enabling Lake Formation governance.
142+
Test creating a FeatureGroupManager and enabling Lake Formation governance.
143143
144144
This test:
145-
1. Creates a new FeatureGroup with offline store
145+
1. Creates a new FeatureGroupManager with offline store
146146
2. Waits for it to reach Created status
147147
3. Enables Lake Formation governance (registers S3, grants permissions, revokes IAM principals)
148-
4. Cleans up the FeatureGroup
148+
4. Cleans up the FeatureGroupManager
149149
"""
150150

151151
fg_name = generate_feature_group_name()
152152
fg = None
153153

154154
try:
155-
# Create the FeatureGroup
155+
# Create the FeatureGroupManager
156156
fg = create_test_feature_group(fg_name, s3_uri, role, region)
157157
assert fg is not None
158158

@@ -179,26 +179,26 @@ def test_create_feature_group_and_enable_lake_formation(s3_uri, role, region):
179179
@pytest.mark.slow_test
180180
def test_create_feature_group_with_lake_formation_enabled(s3_uri, role, region):
181181
"""
182-
Test creating a FeatureGroup with lake_formation_config.enabled=True.
182+
Test creating a FeatureGroupManager with lake_formation_config.enabled=True.
183183
184184
This test verifies the integrated workflow where Lake Formation is enabled
185-
automatically during FeatureGroup creation:
186-
1. Creates a new FeatureGroup with lake_formation_config.enabled=True
187-
2. Verifies the FeatureGroup is created and Lake Formation is configured
188-
3. Cleans up the FeatureGroup
185+
automatically during FeatureGroupManager creation:
186+
1. Creates a new FeatureGroupManager with lake_formation_config.enabled=True
187+
2. Verifies the FeatureGroupManager is created and Lake Formation is configured
188+
3. Cleans up the FeatureGroupManager
189189
"""
190190

191191
fg_name = generate_feature_group_name()
192192
fg = None
193193

194194
try:
195-
# Create the FeatureGroup with Lake Formation enabled
195+
# Create the FeatureGroupManager with Lake Formation enabled
196196

197197
offline_store_config = OfflineStoreConfig(s3_storage_config=S3StorageConfig(s3_uri=s3_uri))
198198
lake_formation_config = LakeFormationConfig()
199199
lake_formation_config.enabled = True
200200

201-
fg = FeatureGroup.create(
201+
fg = FeatureGroupManager.create(
202202
feature_group_name=fg_name,
203203
record_identifier_feature_name="record_id",
204204
event_time_feature_name="event_time",
@@ -208,7 +208,7 @@ def test_create_feature_group_with_lake_formation_enabled(s3_uri, role, region):
208208
lake_formation_config=lake_formation_config,
209209
)
210210

211-
# Verify the FeatureGroup was created
211+
# Verify the FeatureGroupManager was created
212212
assert fg is not None
213213
assert fg.feature_group_name == fg_name
214214
assert fg.feature_group_status == "Created"
@@ -226,24 +226,24 @@ def test_create_feature_group_with_lake_formation_enabled(s3_uri, role, region):
226226
@pytest.mark.serial
227227
def test_create_feature_group_without_lake_formation(s3_uri, role, region):
228228
"""
229-
Test creating a FeatureGroup without Lake Formation enabled.
229+
Test creating a FeatureGroupManager without Lake Formation enabled.
230230
231231
This test verifies that when lake_formation_config is not provided or enabled=False,
232-
the FeatureGroup is created successfully without any Lake Formation operations:
233-
1. Creates a new FeatureGroup without lake_formation_config
234-
2. Verifies the FeatureGroup is created successfully
232+
the FeatureGroupManager is created successfully without any Lake Formation operations:
233+
1. Creates a new FeatureGroupManager without lake_formation_config
234+
2. Verifies the FeatureGroupManager is created successfully
235235
3. Verifies no Lake Formation operations were performed
236-
4. Cleans up the FeatureGroup
236+
4. Cleans up the FeatureGroupManager
237237
"""
238238
fg_name = generate_feature_group_name()
239239
fg = None
240240

241241
try:
242-
# Create the FeatureGroup without Lake Formation
242+
# Create the FeatureGroupManager without Lake Formation
243243
offline_store_config = OfflineStoreConfig(s3_storage_config=S3StorageConfig(s3_uri=s3_uri))
244244

245245
# Create without lake_formation_config (default behavior)
246-
fg = FeatureGroup.create(
246+
fg = FeatureGroupManager.create(
247247
feature_group_name=fg_name,
248248
record_identifier_feature_name="record_id",
249249
event_time_feature_name="event_time",
@@ -252,7 +252,7 @@ def test_create_feature_group_without_lake_formation(s3_uri, role, region):
252252
role_arn=role,
253253
)
254254

255-
# Verify the FeatureGroup was created
255+
# Verify the FeatureGroupManager was created
256256
assert fg is not None
257257
assert fg.feature_group_name == fg_name
258258

@@ -278,7 +278,7 @@ def test_create_feature_group_without_lake_formation(s3_uri, role, region):
278278

279279
def test_create_feature_group_with_lake_formation_fails_without_offline_store(role, region):
280280
"""
281-
Test that creating a FeatureGroup with enable_lake_formation=True fails
281+
Test that creating a FeatureGroupManager with enable_lake_formation=True fails
282282
when no offline store is configured.
283283
284284
Expected behavior: ValueError should be raised indicating offline store is required.
@@ -290,7 +290,7 @@ def test_create_feature_group_with_lake_formation_fails_without_offline_store(ro
290290

291291
# Attempt to create without offline store but with Lake Formation enabled
292292
with pytest.raises(ValueError) as exc_info:
293-
FeatureGroup.create(
293+
FeatureGroupManager.create(
294294
feature_group_name=fg_name,
295295
record_identifier_feature_name="record_id",
296296
event_time_feature_name="event_time",
@@ -307,7 +307,7 @@ def test_create_feature_group_with_lake_formation_fails_without_offline_store(ro
307307

308308
def test_create_feature_group_with_lake_formation_fails_without_role(s3_uri, region):
309309
"""
310-
Test that creating a FeatureGroup with lake_formation_config.enabled=True fails
310+
Test that creating a FeatureGroupManager with lake_formation_config.enabled=True fails
311311
when no role_arn is provided.
312312
313313
Expected behavior: ValueError should be raised indicating role_arn is required.
@@ -320,7 +320,7 @@ def test_create_feature_group_with_lake_formation_fails_without_role(s3_uri, reg
320320

321321
# Attempt to create without role_arn but with Lake Formation enabled
322322
with pytest.raises(ValueError) as exc_info:
323-
FeatureGroup.create(
323+
FeatureGroupManager.create(
324324
feature_group_name=fg_name,
325325
record_identifier_feature_name="record_id",
326326
event_time_feature_name="event_time",
@@ -335,20 +335,20 @@ def test_create_feature_group_with_lake_formation_fails_without_role(s3_uri, reg
335335

336336
def test_enable_lake_formation_fails_for_non_created_status(s3_uri, role, region):
337337
"""
338-
Test that enable_lake_formation() fails when called on a FeatureGroup
338+
Test that enable_lake_formation() fails when called on a FeatureGroupManager
339339
that is not in 'Created' status.
340340
341341
Expected behavior: ValueError should be raised indicating the Feature Group
342342
must be in 'Created' status.
343343
344-
Note: This test creates its own FeatureGroup because it needs to test
344+
Note: This test creates its own FeatureGroupManager because it needs to test
345345
behavior during the 'Creating' status, which requires a fresh resource.
346346
"""
347347
fg_name = generate_feature_group_name()
348348
fg = None
349349

350350
try:
351-
# Create the FeatureGroup
351+
# Create the FeatureGroupManager
352352
fg = create_test_feature_group(fg_name, s3_uri, role, region)
353353
assert fg is not None
354354

@@ -370,22 +370,22 @@ def test_enable_lake_formation_fails_for_non_created_status(s3_uri, role, region
370370

371371
def test_enable_lake_formation_without_offline_store(role, region):
372372
"""
373-
Test that enable_lake_formation() fails when called on a FeatureGroup
373+
Test that enable_lake_formation() fails when called on a FeatureGroupManager
374374
without an offline store configured.
375375
376376
Expected behavior: ValueError should be raised indicating offline store is required.
377377
378-
Note: This test creates a FeatureGroup with only online store, which is a valid
378+
Note: This test creates a FeatureGroupManager with only online store, which is a valid
379379
configuration, but Lake Formation cannot be enabled for it.
380380
"""
381381
fg_name = generate_feature_group_name()
382382
fg = None
383383

384384
try:
385-
# Create a FeatureGroup with only online store (no offline store)
385+
# Create a FeatureGroupManager with only online store (no offline store)
386386
online_store_config = OnlineStoreConfig(enable_online_store=True)
387387

388-
fg = FeatureGroup.create(
388+
fg = FeatureGroupManager.create(
389389
feature_group_name=fg_name,
390390
record_identifier_feature_name="record_id",
391391
event_time_feature_name="event_time",
@@ -477,7 +477,7 @@ def test_enable_lake_formation_full_flow_with_policy_output(s3_uri, role, region
477477
Test the full Lake Formation flow with S3 deny policy output.
478478
479479
This test verifies:
480-
1. Creates a FeatureGroup with offline store
480+
1. Creates a FeatureGroupManager with offline store
481481
2. Enables Lake Formation with show_s3_policy=True
482482
3. Verifies all Lake Formation phases complete successfully
483483
4. Verifies the S3 deny policy is logged
@@ -489,7 +489,7 @@ def test_enable_lake_formation_full_flow_with_policy_output(s3_uri, role, region
489489
fg = None
490490

491491
try:
492-
# Create the FeatureGroup
492+
# Create the FeatureGroupManager
493493
fg = create_test_feature_group(fg_name, s3_uri, role, region)
494494
assert fg is not None
495495

@@ -553,7 +553,7 @@ def test_enable_lake_formation_no_policy_output_by_default(s3_uri, role, region,
553553
Test that S3 deny policy is NOT logged when show_s3_policy=False (default).
554554
555555
This test verifies:
556-
1. Creates a FeatureGroup with offline store
556+
1. Creates a FeatureGroupManager with offline store
557557
2. Enables Lake Formation without show_s3_policy (defaults to False)
558558
3. Verifies all Lake Formation phases complete successfully
559559
4. Verifies the S3 deny policy is NOT logged
@@ -564,7 +564,7 @@ def test_enable_lake_formation_no_policy_output_by_default(s3_uri, role, region,
564564
fg = None
565565

566566
try:
567-
# Create the FeatureGroup
567+
# Create the FeatureGroupManager
568568
fg = create_test_feature_group(fg_name, s3_uri, role, region)
569569
assert fg is not None
570570

@@ -601,7 +601,7 @@ def test_enable_lake_formation_with_custom_role_policy_output(s3_uri, role, regi
601601
Test the full Lake Formation flow with custom registration role and policy output.
602602
603603
This test verifies:
604-
1. Creates a FeatureGroup with offline store
604+
1. Creates a FeatureGroupManager with offline store
605605
2. Enables Lake Formation with use_service_linked_role=False and a custom registration_role_arn
606606
3. Verifies the S3 deny policy uses the custom role ARN instead of service-linked role
607607
@@ -614,7 +614,7 @@ def test_enable_lake_formation_with_custom_role_policy_output(s3_uri, role, regi
614614
fg = None
615615

616616
try:
617-
# Create the FeatureGroup
617+
# Create the FeatureGroupManager
618618
fg = create_test_feature_group(fg_name, s3_uri, role, region)
619619
assert fg is not None
620620

0 commit comments

Comments
 (0)