-
Notifications
You must be signed in to change notification settings - Fork 6
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
clean-up-last-modifier-id: Don't delete guest users #750
Conversation
ef7a91d
to
968802e
Compare
Fixes sqlalchemy.exc.IntegrityError: (raised as a result of Query-invoked autoflush; consider using a session.no_autoflush block if this flush is occurring prematurely) (psycopg2.errors.ForeignKeyViolation) update or delete on table "account" violates foreign key constraint "session_last_modifier_id_fkey" on table "session" DETAIL: Key (id)=(...) is still referenced from table "session". Ref syslabcom/scrum#2372
968802e
to
64ca1ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me we are removing code that we would like to have just because it is hard to fix it.
It does not feel right.
I have no idea if we are removing a feature which is requested by some customer.
I am wondering if adding a session.flush()
or even an intermediate commit
after changing the last_modifier_id
might help.
@pilz do you know if somebody cares?
@reinhardt , I am available to pair on this to find a solution that keeps the feature.
No. The main point is to fix a database state that makes trouble with the guest user cleanup script (#2155). If we don't delete the users here, they will be deleted there: https://github.com/euphorie/osha.oira/blob/main/src/osha/oira/sql_scripts.py#L55 FTR OSHA has some concern about guest users created by bots, but this is about guest sessions that were taken over by registered users and therefore cannot have been created by bots.
I had no succes with that, as mentioned in https://github.com/syslabcom/scrum/issues/2372#issuecomment-2180401673 I have doubts whether it's worth spending more time on this. What do you think @pilz? (So far it's about half a day.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am pretty sure this will never do anything because of plone protect. In #751 I had to add an explicit commit to make it pass the csrf protection.
Does plone.protect also protect postgresql? In any case, closing this in favour of #751. |
Ref syslabcom/scrum#2372