Skip to content

Commit b0c8980

Browse files
committed
refactor(feature-store): Remove client caching from FeatureGroupManager
Remove _lf_client_cache and _s3_client_cache instance caches from _get_lake_formation_client and _get_s3_client. Each call now creates a fresh boto3 client directly. Remove corresponding cache-specific unit tests (cache reuse and different-region tests). --- X-AI-Prompt: remove client caching for lf and s3 in feature_group_manager and update tests X-AI-Tool: kiro-cli
1 parent 481e193 commit b0c8980

2 files changed

Lines changed: 6 additions & 97 deletions

File tree

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

Lines changed: 6 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ class LakeFormationConfig(Base):
4949
class FeatureGroupManager(FeatureGroup):
5050
"""FeatureGroup with extended management capabilities."""
5151

52-
_lf_client_cache: dict
53-
_s3_client_cache: dict
54-
5552
@staticmethod
5653
def _s3_uri_to_arn(s3_uri: str, region: Optional[str] = None) -> str:
5754
"""
@@ -190,11 +187,7 @@ def _get_lake_formation_client(
190187
region: Optional[str] = None,
191188
):
192189
"""
193-
Get a Lake Formation client, reusing a cached client when possible.
194-
195-
The client is cached on the instance keyed by (session, region). Subsequent
196-
calls with the same arguments return the existing client instead of creating
197-
a new one.
190+
Get a Lake Formation client.
198191
199192
Args:
200193
session: Boto3 session. If not provided, a new session will be created.
@@ -203,29 +196,16 @@ def _get_lake_formation_client(
203196
Returns:
204197
A boto3 Lake Formation client.
205198
"""
206-
cache_key = (id(session), region)
207-
if not hasattr(self, "_lf_client_cache") or self._lf_client_cache is None:
208-
self._lf_client_cache = {}
209-
210-
if cache_key not in self._lf_client_cache:
211-
boto_session = session or Session()
212-
self._lf_client_cache[cache_key] = boto_session.client(
213-
"lakeformation", region_name=region
214-
)
215-
216-
return self._lf_client_cache[cache_key]
199+
boto_session = session or Session()
200+
return boto_session.client("lakeformation", region_name=region)
217201

218202
def _get_s3_client(
219203
self,
220204
session: Optional[Session] = None,
221205
region: Optional[str] = None,
222206
):
223207
"""
224-
Get an S3 client, reusing a cached client when possible.
225-
226-
The client is cached on the instance keyed by (session, region). Subsequent
227-
calls with the same arguments return the existing client instead of creating
228-
a new one.
208+
Get an S3 client.
229209
230210
Args:
231211
session: Boto3 session. If not provided, a new session will be created.
@@ -234,17 +214,8 @@ def _get_s3_client(
234214
Returns:
235215
A boto3 S3 client.
236216
"""
237-
cache_key = (id(session), region)
238-
if not hasattr(self, "_s3_client_cache") or self._s3_client_cache is None:
239-
self._s3_client_cache = {}
240-
241-
if cache_key not in self._s3_client_cache:
242-
boto_session = session or Session()
243-
self._s3_client_cache[cache_key] = boto_session.client(
244-
"s3", region_name=region
245-
)
246-
247-
return self._s3_client_cache[cache_key]
217+
boto_session = session or Session()
218+
return boto_session.client("s3", region_name=region)
248219

249220
def _register_s3_with_lake_formation(
250221
self,

sagemaker-mlops/tests/unit/sagemaker/mlops/feature_store/test_feature_group_manager.py

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -70,37 +70,6 @@ def test_creates_client_with_provided_session(self):
7070
mock_session.client.assert_called_with("lakeformation", region_name="us-west-2")
7171
assert client == mock_client
7272

73-
def test_caches_client_for_same_session_and_region(self):
74-
"""Test that repeated calls with the same session and region reuse the cached client."""
75-
mock_session = MagicMock()
76-
mock_client = MagicMock()
77-
mock_session.client.return_value = mock_client
78-
79-
fg = MagicMock(spec=FeatureGroupManager)
80-
fg._get_lake_formation_client = FeatureGroupManager._get_lake_formation_client.__get__(fg)
81-
82-
client1 = fg._get_lake_formation_client(session=mock_session, region="us-west-2")
83-
client2 = fg._get_lake_formation_client(session=mock_session, region="us-west-2")
84-
85-
assert client1 is client2
86-
mock_session.client.assert_called_once()
87-
88-
def test_creates_new_client_for_different_region(self):
89-
"""Test that a different region produces a new client."""
90-
mock_session = MagicMock()
91-
mock_client_west = MagicMock()
92-
mock_client_east = MagicMock()
93-
mock_session.client.side_effect = [mock_client_west, mock_client_east]
94-
95-
fg = MagicMock(spec=FeatureGroupManager)
96-
fg._get_lake_formation_client = FeatureGroupManager._get_lake_formation_client.__get__(fg)
97-
98-
client1 = fg._get_lake_formation_client(session=mock_session, region="us-west-2")
99-
client2 = fg._get_lake_formation_client(session=mock_session, region="us-east-1")
100-
101-
assert client1 is not client2
102-
assert mock_session.client.call_count == 2
103-
10473

10574
class TestGetS3Client:
10675
"""Tests for _get_s3_client method."""
@@ -135,37 +104,6 @@ def test_creates_client_with_provided_session(self):
135104
mock_session.client.assert_called_with("s3", region_name="us-west-2")
136105
assert client == mock_client
137106

138-
def test_caches_client_for_same_session_and_region(self):
139-
"""Test that repeated calls with the same session and region reuse the cached client."""
140-
mock_session = MagicMock()
141-
mock_client = MagicMock()
142-
mock_session.client.return_value = mock_client
143-
144-
fg = MagicMock(spec=FeatureGroupManager)
145-
fg._get_s3_client = FeatureGroupManager._get_s3_client.__get__(fg)
146-
147-
client1 = fg._get_s3_client(session=mock_session, region="us-west-2")
148-
client2 = fg._get_s3_client(session=mock_session, region="us-west-2")
149-
150-
assert client1 is client2
151-
mock_session.client.assert_called_once()
152-
153-
def test_creates_new_client_for_different_region(self):
154-
"""Test that a different region produces a new client."""
155-
mock_session = MagicMock()
156-
mock_client_west = MagicMock()
157-
mock_client_east = MagicMock()
158-
mock_session.client.side_effect = [mock_client_west, mock_client_east]
159-
160-
fg = MagicMock(spec=FeatureGroupManager)
161-
fg._get_s3_client = FeatureGroupManager._get_s3_client.__get__(fg)
162-
163-
client1 = fg._get_s3_client(session=mock_session, region="us-west-2")
164-
client2 = fg._get_s3_client(session=mock_session, region="us-east-1")
165-
166-
assert client1 is not client2
167-
assert mock_session.client.call_count == 2
168-
169107

170108
class TestRegisterS3WithLakeFormation:
171109
"""Tests for _register_s3_with_lake_formation method."""

0 commit comments

Comments
 (0)