Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix send_notification background task #28

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

beenje
Copy link
Collaborator

@beenje beenje commented Feb 26, 2024

We shouldn't pass the session to the background task. get_db() dependency will close the session after the request. There is no warranty the session won't be closed before the background task is done.

Instead we create a local session in the background task.

Avoid:

File "/app/app/utils.py", line 82, in send_notification
    for user_notification in notification.users_notification:
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/sqlalchemy/orm/attributes.py", line 294, in __get__
    return self.impl.get(instance_state(instance), dict_)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/sqlalchemy/orm/attributes.py", line 730, in get
    value = self.callable_(state, passive)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/sqlalchemy/orm/strategies.py", line 717, in _load_for_state
    raise orm_exc.DetachedInstanceError(
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <Notification at 0x7f1f8fb00c50> is not bound to a Session; lazy load operation of attribute 'users_notification' cannot proceed (Background on this error at: http://sqlalche.me/e/13/bhk3)

We shouldn't pass the session to the background task.
get_db() dependency will close the session after the request.
There is no warranty the session won't be closed before the background task is done.

Instead we create a local session in the background task.

Avoid:

File "/app/app/utils.py", line 82, in send_notification
    for user_notification in notification.users_notification:
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/sqlalchemy/orm/attributes.py", line 294, in __get__
    return self.impl.get(instance_state(instance), dict_)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/sqlalchemy/orm/attributes.py", line 730, in get
    value = self.callable_(state, passive)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.11/site-packages/sqlalchemy/orm/strategies.py", line 717, in _load_for_state
    raise orm_exc.DetachedInstanceError(
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <Notification at 0x7f1f8fb00c50> is not bound to a Session; lazy load operation of attribute 'users_notification' cannot proceed (Background on this error at: http://sqlalche.me/e/13/bhk3)
@emanuelelaface emanuelelaface merged commit 83c772f into master Feb 26, 2024
5 checks passed
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.

2 participants