Skip to content

Commit

Permalink
[ci] Push cleanup methods in on_startup to ensure proper execution or…
Browse files Browse the repository at this point in the history
…der (#14172)

The current `on_cleanup` code carefully attempts to close resources in
the correct order (if B depends on A, we should close B before we close
A). Doing so is pretty error prone though and we have messed it up in
the past, leading to noisy error logs when pods are shut down. If we
instead push `.close` methods onto a stack immediately after they are
initialized, the exit stack cannot be executed in the wrong order.
  • Loading branch information
daniel-goldstein authored Jan 18, 2024
1 parent bfb5ec1 commit cff9dc5
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions ci/ci/ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -767,17 +767,24 @@ class AppKeys(CommonAiohttpAppKeys):
FROZEN_MERGE_DEPLOY = web.AppKey('frozen_merge_deploy', bool)
TASK_MANAGER = web.AppKey('task_manager', aiotools.BackgroundTaskManager)
DEVELOPERS = web.AppKey('developers', List[UserData])
EXIT_STACK = web.AppKey('exit_stack', AsyncExitStack)


async def on_startup(app: web.Application):
exit_stack = AsyncExitStack()
app[AppKeys.EXIT_STACK] = exit_stack

client_session = httpx.client_session()
exit_stack.push_async_callback(client_session.close)

app[AppKeys.CLIENT_SESSION] = client_session
app[AppKeys.GH_CLIENT] = gh_aiohttp.GitHubAPI(client_session.client_session, 'ci', oauth_token=oauth_token)
app[AppKeys.BATCH_CLIENT] = await BatchClient.create('ci')
exit_stack.push_async_callback(app[AppKeys.BATCH_CLIENT].close)

app[AppKeys.DB] = Database()
await app[AppKeys.DB].async_init()
exit_stack.push_async_callback(app[AppKeys.DB].async_close)

row = await app[AppKeys.DB].select_and_fetchone(
"""
Expand All @@ -787,6 +794,7 @@ async def on_startup(app: web.Application):

app[AppKeys.FROZEN_MERGE_DEPLOY] = row['frozen_merge_deploy']
app[AppKeys.TASK_MANAGER] = aiotools.BackgroundTaskManager()
exit_stack.callback(app[AppKeys.TASK_MANAGER].shutdown)

if DEFAULT_NAMESPACE == 'default':
kubernetes_asyncio.config.load_incluster_config()
Expand Down Expand Up @@ -817,11 +825,7 @@ async def on_startup(app: web.Application):


async def on_cleanup(app: web.Application):
async with AsyncExitStack() as cleanup:
cleanup.push_async_callback(app[AppKeys.DB].async_close)
cleanup.push_async_callback(app[AppKeys.CLIENT_SESSION].close)
cleanup.push_async_callback(app[AppKeys.BATCH_CLIENT].close)
cleanup.callback(app[AppKeys.TASK_MANAGER].shutdown)
await app[AppKeys.EXIT_STACK].aclose()


def run():
Expand Down

0 comments on commit cff9dc5

Please sign in to comment.