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

migration: make 41157f1933d6_remove_table.py more robust #151

Open
GraemeWatt opened this issue Nov 17, 2022 · 1 comment
Open

migration: make 41157f1933d6_remove_table.py more robust #151

GraemeWatt opened this issue Nov 17, 2022 · 1 comment

Comments

@GraemeWatt
Copy link

We just upgraded the HEPData software from invenio-userprofiles v1.2.1 to v2.0.3 and invenio-accounts 1.4.13 to v2.0.0. Running the alembic upgrade command on (a copy of) the production database gave a couple of errors from this SQL query:

query = (
"UPDATE accounts_user SET "
" username = '{username}', "
" displayname = '{displayname}', "
" profile = '{profile_data_string}', "
" preferences = '{preferences}' "
"WHERE accounts_user.id = {user_id};".format(
username=username,
displayname=displayname,
profile_data_string=json.dumps(profile_data),
preferences=json.dumps(preferences),
user_id=user_id,
)
)

  1. Some local accounts have a null username in the existing userprofiles_userprofile table if it has not been explicitly set by the HEPData user, giving an error message:
sqlalchemy.exc.IntegrityError: (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "uq_accounts_user_username"
DETAIL:  Key (username)=(None) already exists.
[SQL: UPDATE accounts_user SET  username = 'None',  displayname = 'None',  profile = '{"full_name": "Albert Einstein"}',  preferences = '{"visibility": "restricted", "email_visibility": "restricted"}' WHERE accounts_user.id = 1234;]

I worked around this problem by executing:

update userprofiles_userprofile set username=user_id where username is null;

before running the alembic upgrade command.

  1. Some full_name entries in the existing userprofiles_userprofile table can contain a ' symbol, giving an error message:
sqlalchemy.exc.ProgrammingError: (psycopg2.errors.SyntaxError) syntax error at or near "Arc"
LINE 1: ...'jeanne',  profile = '{"full_name": "Jeanne d'Arc"}'...
[SQL: UPDATE accounts_user SET  username = 'jeanne',  displayname = 'jeanne',  profile = '{"full_name": "Jeanne d'Arc"}',  preferences = '{"visibility": "restricted", "email_visibility": "restricted"}' WHERE accounts_user.id = 1234;]

Again, I could work around this problem by removing ' from the full_name in the userprofiles_userprofile table, running alembic upgrade, then restoring the original full_name values for the 4 affected users.

Feel free to close this issue if you think these errors are too specific to be worth fixing, although patching the upgrade command is probably fairly simple.

@GraemeWatt
Copy link
Author

GraemeWatt commented Apr 28, 2023

I found another issue related to this migration which should have been handled by running alembic upgrade.

After the migration, new entries in the accounts_user table of the database have default values of profile={} and preferences={"visibility": "restricted", "email_visibility": "restricted"} (for v2.1.0). However, before the migration, many existing entries in the accounts_user table do not have a corresponding entry in the userprofiles_userprofile table, because the user has never filled in their profile. For the HEPData case, this situation was dominant (5304 of 6960 total users) where, after the migration, the accounts_user table had null entries for the profile and preferences fields. The user's profile cannot subsequently be edited via the web form because attempting to do so gives an exception TypeError: 'NoneType' object is not a mapping from this line:

self._user.user_profile = {**self._user.user_profile, attr: value}

The solution was to run the following SQL command:

update accounts_user set profile='{}', preferences='{"visibility": "restricted", "email_visibility": "restricted"}' where profile is null;

However, this command would be better executed inside the upgrade() function. Alternatively, the code should be modified to tolerate null entries for the profile and preferences fields.

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

1 participant