Skip to content

Commit fafa6b6

Browse files
[async] Review fixes - Fixed not renamed urls
1 parent 9ca88cc commit fafa6b6

File tree

2 files changed

+44
-49
lines changed

2 files changed

+44
-49
lines changed

test/unit/aio/test_retry_network_async.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@
4545
pytestmark = pytest.mark.skipolddriver
4646

4747

48+
# Module and class path constants for easier refactoring
49+
ASYNC_SESSION_MANAGER_MODULE = "snowflake.connector.aio._session_manager"
50+
ASYNC_SESSION_MANAGER = f"{ASYNC_SESSION_MANAGER_MODULE}.SessionManager"
51+
ASYNC_SNOWFLAKE_SSL_CONNECTOR = f"{ASYNC_SESSION_MANAGER_MODULE}.SnowflakeSSLConnector"
4852
THIS_DIR = os.path.dirname(os.path.realpath(__file__))
4953

5054

@@ -392,9 +396,9 @@ async def test_retry_connection_reset_error(caplog):
392396
async def error_send(*args, **kwargs):
393397
raise OSError(104, "ECONNRESET")
394398

395-
with patch(
396-
"snowflake.connector.aio._session_manager.SnowflakeSSLConnector.connect"
397-
) as mock_conn, patch("aiohttp.client_reqrep.ClientRequest.send", error_send):
399+
with patch(f"{ASYNC_SNOWFLAKE_SSL_CONNECTOR}.connect") as mock_conn, patch(
400+
"aiohttp.client_reqrep.ClientRequest.send", error_send
401+
):
398402
with caplog.at_level(logging.DEBUG):
399403
await rest.fetch(timeout=10, **default_parameters)
400404

test/unit/aio/test_session_manager_async.py

Lines changed: 37 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@
1212
)
1313
from snowflake.connector.constants import OCSPMode
1414

15-
HOST_SFC_TEST_0 = "sfctest0.snowflakecomputing.com"
16-
URL_SFC_TEST_0 = f"https://{HOST_SFC_TEST_0}:443/session/v1/login-request"
15+
# Module and class path constants for easier refactoring
16+
ASYNC_SESSION_MANAGER_MODULE = "snowflake.connector.aio._session_manager"
17+
ASYNC_SESSION_MANAGER = f"{ASYNC_SESSION_MANAGER_MODULE}.SessionManager"
1718

18-
HOST_SFC_S3_STAGE = "sfc-ds2-customer-stage.s3.amazonaws.com"
19-
URL_SFC_S3_STAGE_1 = f"https://{HOST_SFC_S3_STAGE}/rgm1-s-sfctest0/stages/"
20-
URL_SFC_S3_STAGE_2 = f"https://{HOST_SFC_S3_STAGE}/rgm1-s-sfctst0/stages/another-url"
19+
TEST_HOST_1 = "testaccount.example.com"
20+
TEST_URL_1 = f"https://{TEST_HOST_1}:443/session/v1/login-request"
21+
22+
TEST_STORAGE_HOST = "test-customer-stage.s3.example.com"
23+
TEST_STORAGE_URL_1 = f"https://{TEST_STORAGE_HOST}/test-stage/stages/"
24+
TEST_STORAGE_URL_2 = f"https://{TEST_STORAGE_HOST}/test-stage/stages/another-url"
2125

2226

2327
async def create_session(
@@ -46,18 +50,17 @@ async def close_and_assert(manager: SessionManager, expected_pool_count: int) ->
4650
ORIGINAL_MAKE_SESSION = SessionManager.make_session
4751

4852

49-
@pytest.mark.asyncio
5053
@mock.patch(
51-
"snowflake.connector.aio._session_manager.SessionManager.make_session",
54+
f"{ASYNC_SESSION_MANAGER}.make_session",
5255
side_effect=ORIGINAL_MAKE_SESSION,
5356
autospec=True,
5457
)
5558
async def test_pooling_disabled(make_session_mock):
5659
"""When pooling is disabled every request creates and closes a new Session."""
5760
manager = SessionManager(use_pooling=False)
5861

59-
await create_session(manager, url=URL_SFC_TEST_0)
60-
await create_session(manager, url=URL_SFC_TEST_0)
62+
await create_session(manager, url=TEST_URL_1)
63+
await create_session(manager, url=TEST_URL_1)
6164

6265
# Two independent sessions were created
6366
assert make_session_mock.call_count == 2
@@ -67,9 +70,8 @@ async def test_pooling_disabled(make_session_mock):
6770
await close_and_assert(manager, expected_pool_count=0)
6871

6972

70-
@pytest.mark.asyncio
7173
@mock.patch(
72-
"snowflake.connector.aio._session_manager.SessionManager.make_session",
74+
f"{ASYNC_SESSION_MANAGER}.make_session",
7375
side_effect=ORIGINAL_MAKE_SESSION,
7476
autospec=True,
7577
)
@@ -79,36 +81,35 @@ async def test_single_hostname_pooling(make_session_mock):
7981

8082
# Create 5 sequential sessions for the same hostname
8183
for _ in range(5):
82-
await create_session(manager, url=URL_SFC_TEST_0)
84+
await create_session(manager, url=TEST_URL_1)
8385

8486
# Only one underlying Session should have been created
8587
assert make_session_mock.call_count == 1
8688

87-
assert list(manager.sessions_map.keys()) == [HOST_SFC_TEST_0]
88-
pool = manager.sessions_map[HOST_SFC_TEST_0]
89+
assert list(manager.sessions_map.keys()) == [TEST_HOST_1]
90+
pool = manager.sessions_map[TEST_HOST_1]
8991
assert len(pool._idle_sessions) == 1
9092
assert len(pool._active_sessions) == 0
9193

9294
await close_and_assert(manager, expected_pool_count=1)
9395

9496

95-
@pytest.mark.asyncio
9697
@mock.patch(
97-
"snowflake.connector.aio._session_manager.SessionManager.make_session",
98+
f"{ASYNC_SESSION_MANAGER}.make_session",
9899
side_effect=ORIGINAL_MAKE_SESSION,
99100
autospec=True,
100101
)
101102
async def test_multiple_hostnames_separate_pools(make_session_mock):
102103
"""Different hostnames (and None) should create separate pools."""
103104
manager = SessionManager()
104105

105-
for url in [URL_SFC_TEST_0, URL_SFC_S3_STAGE_1, None]:
106+
for url in [TEST_URL_1, TEST_STORAGE_URL_1, None]:
106107
await create_session(manager, num_sessions=2, url=url)
107108

108-
# Two sessions created for each of the three keys (HOST_SFC_TEST_0, HOST_SFC_S3_STAGE, None)
109+
# Two sessions created for each of the three keys (TEST_HOST_1, TEST_STORAGE_HOST, None)
109110
assert make_session_mock.call_count == 6
110111

111-
for expected_host in [HOST_SFC_TEST_0, HOST_SFC_S3_STAGE, None]:
112+
for expected_host in [TEST_HOST_1, TEST_STORAGE_HOST, None]:
112113
assert expected_host in manager.sessions_map
113114

114115
for pool in manager.sessions_map.values():
@@ -118,26 +119,25 @@ async def test_multiple_hostnames_separate_pools(make_session_mock):
118119
await close_and_assert(manager, expected_pool_count=3)
119120

120121

121-
@pytest.mark.asyncio
122122
@mock.patch(
123-
"snowflake.connector.aio._session_manager.SessionManager.make_session",
123+
f"{ASYNC_SESSION_MANAGER}.make_session",
124124
side_effect=ORIGINAL_MAKE_SESSION,
125125
autospec=True,
126126
)
127127
async def test_reuse_sessions_within_pool(make_session_mock):
128128
"""After many sequential sessions only one Session per hostname should exist."""
129129
manager = SessionManager()
130130

131-
for url in [URL_SFC_TEST_0, URL_SFC_S3_STAGE_1, URL_SFC_S3_STAGE_2, None]:
131+
for url in [TEST_URL_1, TEST_STORAGE_URL_1, TEST_STORAGE_URL_2, None]:
132132
for _ in range(10):
133133
await create_session(manager, url=url)
134134

135-
# One Session per unique hostname (URL_SFC_S3_STAGE_2 shares HOST_SFC_S3_STAGE)
135+
# One Session per unique hostname (TEST_STORAGE_URL_2 shares TEST_STORAGE_HOST)
136136
assert make_session_mock.call_count == 3
137137

138138
assert set(manager.sessions_map.keys()) == {
139-
HOST_SFC_TEST_0,
140-
HOST_SFC_S3_STAGE,
139+
TEST_HOST_1,
140+
TEST_STORAGE_HOST,
141141
None,
142142
}
143143
for pool in manager.sessions_map.values():
@@ -147,31 +147,29 @@ async def test_reuse_sessions_within_pool(make_session_mock):
147147
await close_and_assert(manager, expected_pool_count=3)
148148

149149

150-
@pytest.mark.asyncio
151150
async def test_clone_independence():
152151
"""`clone` should return an independent manager sharing only the connector_factory."""
153152
manager = SessionManager()
154-
async with manager.use_session(URL_SFC_TEST_0):
153+
async with manager.use_session(TEST_URL_1):
155154
pass
156-
assert HOST_SFC_TEST_0 in manager.sessions_map
155+
assert TEST_HOST_1 in manager.sessions_map
157156

158157
clone = manager.clone()
159158

160159
assert clone is not manager
161160
assert clone.connector_factory is manager.connector_factory
162161
assert clone.sessions_map == {}
163162

164-
async with clone.use_session(URL_SFC_S3_STAGE_1):
163+
async with clone.use_session(TEST_STORAGE_URL_1):
165164
pass
166165

167-
assert HOST_SFC_S3_STAGE in clone.sessions_map
168-
assert HOST_SFC_S3_STAGE not in manager.sessions_map
166+
assert TEST_STORAGE_HOST in clone.sessions_map
167+
assert TEST_STORAGE_HOST not in manager.sessions_map
169168

170169
await manager.close()
171170
await clone.close()
172171

173172

174-
@pytest.mark.asyncio
175173
async def test_connector_factory_creates_sessions():
176174
"""Verify that connector factory creates aiohttp sessions with proper connector."""
177175
manager = SessionManager()
@@ -185,7 +183,6 @@ async def test_connector_factory_creates_sessions():
185183
await session.close()
186184

187185

188-
@pytest.mark.asyncio
189186
async def test_clone_independent_pools():
190187
"""A clone must *not* share its SessionPool objects with the original."""
191188
base = SessionManager(
@@ -214,7 +211,6 @@ async def test_clone_independent_pools():
214211
await clone.close()
215212

216213

217-
@pytest.mark.asyncio
218214
async def test_config_propagation():
219215
"""Verify that config values are properly propagated to sessions."""
220216
config = AioHttpConfig(
@@ -237,7 +233,6 @@ async def test_config_propagation():
237233
await session.close()
238234

239235

240-
@pytest.mark.asyncio
241236
async def test_config_copy_with():
242237
"""Test that copy_with creates a new config with overrides."""
243238
original_config = AioHttpConfig(
@@ -262,7 +257,6 @@ async def test_config_copy_with():
262257
assert new_config.snowflake_ocsp_mode == OCSPMode.FAIL_CLOSED
263258

264259

265-
@pytest.mark.asyncio
266260
async def test_from_config():
267261
"""Test creating SessionManager from existing config."""
268262
config = AioHttpConfig(
@@ -281,15 +275,14 @@ async def test_from_config():
281275
assert manager2.config.trust_env is False # original value preserved
282276

283277

284-
@pytest.mark.asyncio
285278
async def test_session_pool_lifecycle():
286279
"""Test that session pool properly manages session lifecycle."""
287280
manager = SessionManager(use_pooling=True)
288281

289282
# Get a session - should create new one
290-
async with manager.use_session(URL_SFC_TEST_0):
291-
assert HOST_SFC_TEST_0 in manager.sessions_map
292-
pool = manager.sessions_map[HOST_SFC_TEST_0]
283+
async with manager.use_session(TEST_URL_1):
284+
assert TEST_HOST_1 in manager.sessions_map
285+
pool = manager.sessions_map[TEST_HOST_1]
293286
assert len(pool._active_sessions) == 1
294287
assert len(pool._idle_sessions) == 0
295288

@@ -298,14 +291,13 @@ async def test_session_pool_lifecycle():
298291
assert len(pool._idle_sessions) == 1
299292

300293
# Reuse the same session
301-
async with manager.use_session(URL_SFC_TEST_0):
294+
async with manager.use_session(TEST_URL_1):
302295
assert len(pool._active_sessions) == 1
303296
assert len(pool._idle_sessions) == 0
304297

305298
await manager.close()
306299

307300

308-
@pytest.mark.asyncio
309301
async def test_config_immutability():
310302
"""Test that AioHttpConfig is immutable (frozen dataclass)."""
311303
config = AioHttpConfig(
@@ -327,7 +319,6 @@ async def test_config_immutability():
327319
assert new_config.trust_env is False
328320

329321

330-
@pytest.mark.asyncio
331322
async def test_pickle_session_manager():
332323
"""Test that SessionManager can be pickled and unpickled."""
333324
import pickle
@@ -339,7 +330,7 @@ async def test_pickle_session_manager():
339330
manager = SessionManager(config)
340331

341332
# Create some sessions
342-
async with manager.use_session(URL_SFC_TEST_0):
333+
async with manager.use_session(TEST_URL_1):
343334
pass
344335

345336
# Pickle and unpickle (sessions are discarded during pickle)
@@ -350,8 +341,8 @@ async def test_pickle_session_manager():
350341
assert unpickled.config.trust_env is False
351342
assert unpickled.use_pooling is True
352343
# Pool structure preserved but sessions are empty after unpickling
353-
assert HOST_SFC_TEST_0 in unpickled.sessions_map
354-
pool = unpickled.sessions_map[HOST_SFC_TEST_0]
344+
assert TEST_HOST_1 in unpickled.sessions_map
345+
pool = unpickled.sessions_map[TEST_HOST_1]
355346
assert len(pool._idle_sessions) == 0
356347
assert len(pool._active_sessions) == 0
357348

0 commit comments

Comments
 (0)