-
-
Notifications
You must be signed in to change notification settings - Fork 65
Open
Description
Summary
The delete_many method on SQLAlchemyAsyncRepositoryService currently only accepts a list of primary key IDs. It would be helpful if it could also accept model instances and automatically extract their IDs.
Current Behavior
# This works
await service.delete_many([uuid1, uuid2, uuid3])
# This fails with: invalid input syntax for type uuid: "<Model object at 0x...>"
expired_tokens = await service.list(Model.expires_at < now)
await service.delete_many(list(expired_tokens)) # Passes model objectsProposed Behavior
# Both should work
await service.delete_many([uuid1, uuid2, uuid3]) # IDs
await service.delete_many(list(expired_tokens)) # Model instancesImplementation Suggestion
In the service's delete_many method, check if items are model instances and extract their primary keys:
async def delete_many(
self,
item_ids: list[ModelT] | list[Any],
...
) -> Sequence[ModelT]:
# If items are model instances, extract their IDs
if item_ids and hasattr(item_ids[0], '__table__'):
pk_col = inspect(self.model_type).primary_key[0].name
item_ids = [getattr(item, pk_col) for item in item_ids]
# ... rest of implementationUse Case
This came up when implementing a cleanup job for expired tokens:
async def cleanup_expired_tokens(self) -> int:
expired_tokens = await self.list(
(Model.expires_at < current_time)
| (Model.revoked_at.is_not(None) & Model.revoked_at < current_time)
)
if not expired_tokens:
return 0
# Currently requires this extra step:
token_ids = [token.id for token in expired_tokens]
await self.delete_many(token_ids)
# Would be nicer to just do:
# await self.delete_many(list(expired_tokens))
return len(expired_tokens)Additional Context
This would make the API more forgiving and consistent with patterns where you query models and then want to delete them in bulk.
Metadata
Metadata
Assignees
Labels
No labels