Task collection broken with sqlite? #661
Assignees
Comments
tcompa
added
bug
|
(likely a consequence of #651) |
tcompa
added a commit
that referenced
this issue
May 2, 2023
|
The local CI (using sqlite) passes again after adding d366117 (which was there in some older versions), but the reason is unclear (the rationale behind those few lines of code is not clear, and there is no useful comment). We'll need to understand the issue better before merging. |
|
I saw the same issue when testing things in WSL here. |
|
More info:
Based on some other failure examples (and on my experience), this bug is not fully reproducible - as it depends on the timing of two operations which can vary in different tests. This is why it was not clear to me what the latest working version of fractal-server is. Still, it's most likely a consequence of #651, for the moment the latest working version (with sqlite) is 1.2.2. We are now looking for a reasonable fix. |
|
Other relevant references:
Notable quote (by fastapi maintainer):
|
tcompa
added a commit
that referenced
this issue
May 3, 2023
1. Use context manager for db session 2. Switch to sync db session
|
Minimal failing example: from fastapi import APIRouter
from fastapi import BackgroundTasks
from fastapi import Depends
from devtools import debug
from ...db import AsyncSession
from ...db import get_db
from ...models import State
from ....logger import set_logger
logger = set_logger(__name__)
router = APIRouter()
async def myfun(random: AsyncSession):
logger.critical("BGTASK START")
new_db = await get_db().__anext__()
state = await new_db.get(State, 1)
debug(state)
state.data = {"a": "b"}
logger.critical("BGTASK MERGE:")
await new_db.merge(state)
logger.critical("BGTASK COMMIT:")
await new_db.commit()
await new_db.close()
logger.critical("BGTASK END")
return
@router.post('/test')
async def dummy_endpoint(
background_tasks: BackgroundTasks,
db: AsyncSession = Depends(get_db),
):
logger.critical("ENDPOINT START")
debug(db)
sss = State()
db.add(sss)
await db.commit()
await db.close()
logger.critical("ENDPOINT END")
background_tasks.add_task(myfun, 1)
return |
tcompa
added a commit
that referenced
this issue
May 3, 2023
2 tasks
tcompa
added a commit
that referenced
this issue
May 8, 2023
|
As part of #664, we (me and @mfranzon) introduced a minimal test that captures the wrong behavior, namely an endpoint which submits a background task which acts on the db. The background task can act on the DB either through sync or async sessions, with the async ones being trickier. Note that we currently have two background tasks, in fractal-server:
The difference in what kind of db sessions to use is due to no special reason, and in principle we could make them homogeneous. After reproducing the bug, we introduced two main changes:
With these two changes, CI (including the new specific test) is passing both with sqlite and postgresql. We'll then test this by hand as soon as we have a patch release. |
tcompa
added a commit
that referenced
this issue
May 8, 2023
…und_task Use StaticPool, for sqlite engine (ref #661)
github-project-automation
bot
moved this from TODO
to Done
in Fractal Project Management
|
I just tested More verbose logs: |
github-project-automation
bot
moved this from Done
to TODO
in Fractal Project Management
This was referenced May 9, 2023
github-project-automation
bot
moved this from TODO
to Done
in Fractal Project Management
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Version
1.2.4seems to introduce an issue with task collection, when using sqlite:The text was updated successfully, but these errors were encountered: