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

Change session.expire_all() to session.expunge_all() in delete_cascaded() #163

Open
rsyring opened this issue Dec 12, 2021 · 2 comments
Open

Comments

@rsyring
Copy link
Member

rsyring commented Dec 12, 2021

Expire all leaves the entities that are in the session in the session. That likely increases memory usage and doesn't seem to have an advantage since delete_cascaded() is often called during test setup when we basically want everything reset. .expunge_all() seems like the better choice, I just didn't know about that method when I wrote .delete_cascaded() originally.

@guruofgentoo
Copy link
Member

Might want to think about this a bit more. We have a lot of tests that assume objects are still in the session, even if delete_cascaded was called subsequently. E.g. setup_class creates objects for use in many tests, but setup calls delete_cascaded on an unrelated entity. Those objects are no longer associated with the db session, so making this change will require quite a bit of cleanup.

@rsyring
Copy link
Member Author

rsyring commented Mar 4, 2022

Agreed the proposed solution, changing the default, probably isn't the way to deal with this. In another project, I created a "delete everything" function which used expunge_all() internally and it caused some problems like you described.

At the same time, it probably is worth thinking about the session a bit more, realizing it's also an object cache, and figuring out when it would be appropriate to clear that or just start a new session. Then again, this could be premature optimization since it's never really caused a problem that I know of. I'm just thinking about these kinds of things more since I'm using the "no automatic queries" paradigm more, and really liking it, and sometimes the objects in the session are something you have to think more about using that approach.

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

No branches or pull requests

2 participants