Skip to content

fix: await async def setup() coroutine instead of silently dropping it#2921

Merged
michaeldwan merged 1 commit intomainfrom
md/fix-async-setup
Apr 9, 2026
Merged

fix: await async def setup() coroutine instead of silently dropping it#2921
michaeldwan merged 1 commit intomainfrom
md/fix-async-setup

Conversation

@michaeldwan
Copy link
Copy Markdown
Member

When a predictor has async def setup(), coglet calls it but doesn't await the coroutine. Setup appears to succeed -- logs say "Setup complete" and "Server ready" -- but none of the setup code actually executes. Every prediction then fails with AttributeError because attributes were never set.

call_method0("setup") returns a coroutine object when setup is async. The ? only checks for Python exceptions, so the coroutine was silently dropped.

predict() and train() don't have this problem because load() calls detect_async() for both and call_method_raw() runs the coroutine with asyncio.run(). That pattern was never applied to setup().

The fix: detect whether setup() is async during load(), capture the return value in setup(), and run it with asyncio.run() when it's a coroutine.

Three integration tests cover async setup with async predict, sync predict, and the weights parameter path.

Fixes #2919

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 9, 2026

I'm Bonk, and I've done a quick review of your PR.

This PR fixes a critical bug where async def setup() coroutines were silently dropped instead of being awaited, causing predictions to fail with AttributeError because setup attributes were never set.

The fix follows the same pattern used for predict() and train():

  1. Detect async setup during load() using detect_async()
  2. Store result from setup() call
  3. Conditionally run with asyncio.run() when setup_is_async is true

LGTM

github run

@michaeldwan michaeldwan marked this pull request as ready for review April 9, 2026 17:22
@michaeldwan michaeldwan requested a review from a team as a code owner April 9, 2026 17:22
Copy link
Copy Markdown
Contributor

@markphelps markphelps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

When a predictor has async def setup(), call_method0("setup") returns a
coroutine object. The ? operator only checks for Python exceptions, so
the coroutine was silently dropped -- setup appeared to succeed but none
of the setup code actually executed.

Detect whether setup() is async during load() (matching predict/train),
and run the coroutine with asyncio.run() when it is.

Fixes #2919
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 9, 2026

LGTM

github run

@michaeldwan michaeldwan enabled auto-merge April 9, 2026 18:36
@michaeldwan michaeldwan added this pull request to the merge queue Apr 9, 2026
Merged via the queue into main with commit a780e5e Apr 9, 2026
37 checks passed
@michaeldwan michaeldwan deleted the md/fix-async-setup branch April 9, 2026 18:50
markphelps added a commit that referenced this pull request Apr 13, 2026
…to test-harness-go

* 'test-harness-go' of https://github.com/replicate/cog:
  fix: run async setup() on the same event loop as predict() (#2927)
  fix: support dict and list[dict] as predictor input types (#2928)
  fix: docs deploy workflow uses Go 1.23, can't parse go.mod tool directive (#2924)
  fix: await async def setup() coroutine instead of silently dropping it (#2921)
  chore(deps): bump actions/setup-go from 5 to 6 (#2767)
  fix: stub generation fails locally and produces unformatted output (#2920)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

async def setup() silently fails -- coroutine never awaited

2 participants