Skip to content

Commit 4b3bbb2

Browse files
Bernd VerstCopilot
andcommitted
refactor: hoist stale-loop comprehension into a named local
Address review feedback on the stale-lock cleanup in AsyncAccessTokenManager._get_refresh_lock(). The list comprehension was embedded directly in the `for` statement, which read awkwardly across the wrapped line. Hoist it into a named `stale_loops` local and iterate over that. This is purely a readability change with no behavioural difference: the snapshot is still materialized before mutating the mapping, which is what makes deleting during iteration safe. Note this was not a lint violation - flake8 passes on the file both before and after the change. Also align the async deferred-acquisition test with the naming and assertions that PR #199 introduced for the synchronous manager, and assert explicitly that constructing AsyncAccessTokenManager performs no credential call. The async manager has always deferred its first acquisition, so the cold-start burst flows through the same refresh guard as a later refresh; the test now states that intent directly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bb08d0ed-484d-4015-b905-3e164f68678b
1 parent 76e8f0a commit 4b3bbb2

2 files changed

Lines changed: 10 additions & 3 deletions

File tree

durabletask-azuremanaged/durabletask/azuremanaged/internal/access_token_manager.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,10 @@ def _get_refresh_lock(self) -> asyncio.Lock:
8686
if lock is None:
8787
# Discard locks belonging to loops that are no longer usable so the
8888
# mapping does not grow without bound.
89-
for stale_loop in [
90-
existing for existing in self._refresh_locks if existing.is_closed()]:
89+
stale_loops = [
90+
existing for existing in self._refresh_locks if existing.is_closed()
91+
]
92+
for stale_loop in stale_loops:
9193
del self._refresh_locks[stale_loop]
9294
lock = asyncio.Lock()
9395
self._refresh_locks[loop] = lock

tests/durabletask-azuremanaged/test_durabletask_grpc_interceptor.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,15 @@ async def get_token(self, _scope):
266266

267267

268268
class TestAsyncAccessTokenManagerSingleFlight(unittest.IsolatedAsyncioTestCase):
269-
async def test_concurrent_cold_start_performs_single_acquisition(self):
269+
async def test_deferred_first_acquisition_performs_single_acquisition(self):
270270
credential = _TestAsyncTokenCredential()
271271
manager = AsyncAccessTokenManager(credential)
272272

273+
self.assertEqual(0, credential.calls, "Constructing the manager must not acquire a token")
274+
275+
# The async manager has always deferred the first acquisition (a constructor
276+
# cannot await), so the cold-start burst goes through the same refresh guard
277+
# as a later refresh and must likewise be single-flight.
273278
tokens = await asyncio.gather(*(manager.get_access_token() for _ in range(16)))
274279

275280
self.assertEqual(1, credential.calls)

0 commit comments

Comments
 (0)