Skip to content

fix users multiple imports#62

Draft
utnapischtim wants to merge 1 commit intoinveniosoftware:masterfrom
utnapischtim:fix-users-multiple-imports
Draft

fix users multiple imports#62
utnapischtim wants to merge 1 commit intoinveniosoftware:masterfrom
utnapischtim:fix-users-multiple-imports

Conversation

@utnapischtim
Copy link
Copy Markdown

@utnapischtim utnapischtim commented Sep 14, 2024

  • fix: missing no_autoflush prevents users imports

this should fix inveniosoftware/invenio-users-resources#144

* without the no_autoflush the first() method call initiates a autoflush
  which calls the session.flush() method. this flushes the models in the
  session. this prevents then that other packages could use the
  pre_commit hook to collect all not flushed models and possible index
  them.
@utnapischtim utnapischtim marked this pull request as draft September 26, 2024 20:28
@utnapischtim utnapischtim reopened this Mar 19, 2025
@utnapischtim utnapischtim added this to v14 Mar 19, 2025
@utnapischtim utnapischtim moved this to Triage in v14 Mar 19, 2025
@egabancho
Copy link
Copy Markdown
Member

Why add no_autoflush only to get_user_by_email?

@utnapischtim
Copy link
Copy Markdown
Author

this was a try to fix a bug which uni münster encountered, described here: inveniosoftware/invenio-users-resources#144

it would be better to add no_autoflush also on the other parts.

the real problem is that the autoflush flushes the info to the database which prevents https://github.com/inveniosoftware/invenio-users-resources/blob/19433930ac9581b024db71136a643833255d99af/invenio_users_resources/records/hooks.py#L27 and https://github.com/inveniosoftware/invenio-users-resources/blob/19433930ac9581b024db71136a643833255d99af/invenio_users_resources/records/hooks.py#L61 from really doing what they meant to do.

because flush, removes entry from the .new list, which means that hooking into the session only applies the hook to the latest entry.

therefore all find functions should use no_autoflush but there were some strange sideeffects. so this would have to be debugged a little bit further

@egabancho
Copy link
Copy Markdown
Member

I was working on User-Resources and had a "situation" with the session not behaving correctly. Somehow, my brain connected the dots and remembered you mentioned this some time ago 😂

@utnapischtim
Copy link
Copy Markdown
Author

i added it to the v13 board, so we can discuss this PR. maybe also in Hamburg

@SarahW91
Copy link
Copy Markdown

SarahW91 commented Jun 5, 2025

Will this fix/a similar one be part of v13?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

Reindexing of multiple users after database commit is not triggered

3 participants