From 31552963c4144a9da724b760e7e8a083fd998f11 Mon Sep 17 00:00:00 2001 From: Fabio Manganiello Date: Fri, 10 Mar 2023 12:06:36 +0100 Subject: [PATCH] `EntitiesDb.upsert` should return a deep copy of the upserted entities. Not the upserted entities themselves, no matter if expunged or made transient. Reminder to my future self: returning the flushed entities and then using them outside of the session or in another thread opens a big can of worms when using SQLAlchemy. --- platypush/entities/_engine/repo/db.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/platypush/entities/_engine/repo/db.py b/platypush/entities/_engine/repo/db.py index 7dbc6a07..4eeba786 100644 --- a/platypush/entities/_engine/repo/db.py +++ b/platypush/entities/_engine/repo/db.py @@ -3,7 +3,6 @@ from dataclasses import dataclass from typing import Dict, Iterable, List, Tuple from sqlalchemy import and_, or_ -from sqlalchemy.exc import InvalidRequestError from sqlalchemy.orm import Session from platypush.context import get_plugin @@ -179,18 +178,12 @@ class EntitiesDb: session.commit() - all_entities = list( + # Make a copy of the entities in the batch, so they can be used outside + # of this session/thread without the DetachedInstanceError pain + return list( { - entity.entity_key: entity for batch in batches for entity in batch + entity.entity_key: entity.copy() + for batch in batches + for entity in batch }.values() ) - - # Remove all the entities from the existing session, so they can be - # accessed outside of this context - for e in all_entities: - try: - session.expunge(e) - except InvalidRequestError: - pass - - return all_entities