#788 Fix database migrations issue wherein running 'flask db migrate' produces a lot of unexpected output

[kalbfled]

Issue

This was originally vanotify-team issue 224.

#788

Checklist for db migration commands.

• alter_column
• drop_column
• create_unique_constraint
• drop_constraint
• drop_entity

[ianperera]

Identified the unexpected outputs and spoke to @k-macmillan about this and he pointed me in the right direction.

• I will make a list of all the f-keys for a discussion if we should keep or remove them.

[ianperera]

there are about 51 f keys affected. I'm working on the list for discussion.

[ianperera]

• list of commands that needs to be fixed
  alter_column
  drop_column
  create_unique_constraint
  drop_constraint
  drop_entity

• working on the tables and identifying if the f-keys are relevant.

[ianperera]

Here are list of tables that will be affected by per command:

• alter_column: ft_notification_status, organisation, permissions, services, services_history, templates, templates_history, va_profile_local_cache
• drop_column: ft_notification_status, permissions, user_to_organisation, users
• drop_constraint: users_idp_ids
• create_foreign_key: annual_billing, services, api_keys, users, complaints, notification_history, organisation, domain, fido2_keys, fido2_sessions, inbound_numbers, inbound_sms, invited_organisation_users, invited_users, jobs, login_events, notifications, templates_history, email_branding, organisation_types, letter_branding, provider_details, provider_details_history, service_callback, service_data_retention, service_email_branding, service_email_reply_to, service_letter_branding, service_letter_contacts, service_permissions, service_sms_senders, service_whitelist, provider_details, provider_details, template_folder, template_folder_map, template_redacted, template_redacted, communication_items, user_folder_permissions, user_to_service, user_to_organisation, auth_type, users_idp_ids, verify_codes

[ianperera]

create_foreign_key table relations

   op.create_foreign_key(op.f('fk_annual_billing_service_id_services'), 'annual_billing', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_api_keys_service_id_services'), 'api_keys', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_api_keys_created_by_id_users'), 'api_keys', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_complaints_notification_id_notification_history'), 'complaints', 'notification_history', ['notification_id'], ['id'])
    op.create_foreign_key(op.f('fk_complaints_service_id_services'), 'complaints', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_domain_organisation_id_organisation'), 'domain', 'organisation', ['organisation_id'], ['id'])
    op.create_foreign_key(op.f('fk_fido2_keys_user_id_users'), 'fido2_keys', 'users', ['user_id'], ['id'])
    op.create_foreign_key(op.f('fk_fido2_sessions_user_id_users'), 'fido2_sessions', 'users', ['user_id'], ['id'])
    op.create_foreign_key(op.f('fk_inbound_numbers_service_id_services'), 'inbound_numbers', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_inbound_sms_service_id_services'), 'inbound_sms', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_invited_organisation_users_invited_by_id_users'), 'invited_organisation_users', 'users', ['invited_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_invited_organisation_users_organisation_id_organisation'), 'invited_organisation_users', 'organisation', ['organisation_id'], ['id'])
    op.create_foreign_key(op.f('fk_invited_users_user_id_users'), 'invited_users', 'users', ['user_id'], ['id'])
    op.create_foreign_key(op.f('fk_invited_users_service_id_services'), 'invited_users', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_jobs_template_id_templates'), 'jobs', 'templates', ['template_id'], ['id'])
    op.create_foreign_key(op.f('fk_jobs_created_by_id_users'), 'jobs', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_jobs_service_id_services'), 'jobs', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_login_events_user_id_users'), 'login_events', 'users', ['user_id'], ['id'])
    op.create_foreign_key(op.f('fk_notifications_template_id_templates_history'), 'notifications', 'templates_history', ['template_id', 'template_version'], ['id', 'version'])
    op.create_foreign_key(op.f('fk_notifications_created_by_id_users'), 'notifications', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_notifications_service_id_services'), 'notifications', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_organisation_email_branding_id_email_branding'), 'organisation', 'email_branding', ['email_branding_id'], ['id'])
    op.create_foreign_key(op.f('fk_organisation_organisation_type_organisation_types'), 'organisation', 'organisation_types', ['organisation_type'], ['name'])
    op.create_foreign_key(op.f('fk_organisation_letter_branding_id_letter_branding'), 'organisation', 'letter_branding', ['letter_branding_id'], ['id'])
    op.create_foreign_key(op.f('fk_organisation_agreement_signed_by_id_users'), 'organisation', 'users', ['agreement_signed_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_provider_details_created_by_id_users'), 'provider_details', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_provider_details_history_created_by_id_users'), 'provider_details_history', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_callback_updated_by_id_users'), 'service_callback', 'users', ['updated_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_callback_service_id_services'), 'service_callback', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_data_retention_service_id_services'), 'service_data_retention', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_email_branding_service_id_services'), 'service_email_branding', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_email_reply_to_service_id_services'), 'service_email_reply_to', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_letter_branding_service_id_services'), 'service_letter_branding', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_letter_contacts_service_id_services'), 'service_letter_contacts', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_permissions_service_id_services'), 'service_permissions', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_sms_senders_service_id_services'), 'service_sms_senders', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_service_whitelist_service_id_services'), 'service_whitelist', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_services_created_by_id_users'), 'services', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_services_go_live_user_id_users'), 'services', 'users', ['go_live_user_id'], ['id'])
    op.create_foreign_key(op.f('fk_services_email_provider_id_provider_details'), 'services', 'provider_details', ['email_provider_id'], ['id'])
    op.create_foreign_key(op.f('fk_services_organisation_id_organisation'), 'services', 'organisation', ['organisation_id'], ['id'])
    op.create_foreign_key(op.f('fk_services_sms_provider_id_provider_details'), 'services', 'provider_details', ['sms_provider_id'], ['id'])
    op.create_foreign_key(op.f('fk_services_organisation_type_organisation_types'), 'services', 'organisation_types', ['organisation_type'], ['name'])
    op.create_foreign_key(op.f('fk_template_folder_service_id_services'), 'template_folder', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_template_folder_map_template_id_templates'), 'template_folder_map', 'templates', ['template_id'], ['id'])
    op.create_foreign_key(op.f('fk_template_redacted_updated_by_id_users'), 'template_redacted', 'users', ['updated_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_template_redacted_template_id_templates'), 'template_redacted', 'templates', ['template_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_service_id_services'), 'templates', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_service_letter_contact_id_service_letter_contacts'), 'templates', 'service_letter_contacts', ['service_letter_contact_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_communication_item_id_communication_items'), 'templates', 'communication_items', ['communication_item_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_provider_id_provider_details'), 'templates', 'provider_details', ['provider_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_process_type_template_process_type'), 'templates', 'template_process_type', ['process_type'], ['name'])
    op.create_foreign_key(op.f('fk_templates_created_by_id_users'), 'templates', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_service_id_services'), 'templates', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_service_letter_contact_id_service_letter_contacts'), 'templates', 'service_letter_contacts', ['service_letter_contact_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_communication_item_id_communication_items'), 'templates', 'communication_items', ['communication_item_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_provider_id_provider_details'), 'templates', 'provider_details', ['provider_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_process_type_template_process_type'), 'templates', 'template_process_type', ['process_type'], ['name'])
    op.create_foreign_key(op.f('fk_templates_created_by_id_users'), 'templates', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_history_communication_item_id_communication_items'), 'templates_history', 'communication_items', ['communication_item_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_history_service_id_services'), 'templates_history', 'services', ['service_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_history_service_letter_contact_id_service_letter_contacts'), 'templates_history', 'service_letter_contacts', ['service_letter_contact_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_history_process_type_template_process_type'), 'templates_history', 'template_process_type', ['process_type'], ['name'])
    op.create_foreign_key(op.f('fk_templates_history_created_by_id_users'), 'templates_history', 'users', ['created_by_id'], ['id'])
    op.create_foreign_key(op.f('fk_templates_history_provider_id_provider_details'), 'templates_history', 'provider_details', ['provider_id'], ['id'])
    op.create_foreign_key(op.f('fk_user_folder_permissions_user_id_user_to_service'), 'user_folder_permissions', 'user_to_service', ['user_id', 'service_id'], ['user_id', 'service_id'])
    op.create_foreign_key(op.f('fk_users_auth_type_auth_type'), 'users', 'auth_type', ['auth_type'], ['name'])
    op.create_foreign_key(op.f('fk_users_idp_ids_user_id_users'), 'users_idp_ids', 'users', ['user_id'], ['id'], ondelete='cascade')
    op.create_foreign_key(op.f('fk_verify_codes_user_id_users'), 'verify_codes', 'users', ['user_id'], ['id'])

[ianperera]

@k-macmillan I found the new things while I am working on fixing drop_column.
For example, these are auto generated output revision

op.drop_column('users', 'provider')
op.drop_column('users', 'uid')

I tried to search git blames/history in notification-api but couldn't find out any of related things. But I realized this database is also used in notification-portal and there was a user model defining provider and uid. So I think most of unexpected revisions scripts were generated by differences between model.py of notification-api and the database updated by model of notification-portal (rails migrations).

[ianperera]

op.create_foreign_key(None, 'complaints', 'notification_history', ['notification_id'], ['id'])
op.create_unique_constraint('uix_service_callback_channel', 'service_callback', ['service_id', 'callback_channel'])
op.alter_column('services_history', 'prefix_sms',
               existing_type=sa.BOOLEAN(),
               nullable=False,
               existing_server_default=sa.text('true'))
op.create_foreign_key(None, 'templates_history', 'communication_items', ['communication_item_id'], ['id'])
op.create_foreign_key(None, 'templates_history', 'provider_details', ['provider_id'], ['id'])
op.drop_constraint('users_identity_provider_user_id', 'users', type_='unique')
op.alter_column('va_profile_local_cache', 'source_datetime',
               existing_type=postgresql.TIMESTAMP(),
               nullable=False)

The revision reduced from the 651 lines commands to 7 commands as above when we use the standalone database for notification-api only. So resolving above commands will be good to go.

[ianperera]

@k-macmillan As you can see above revision, there is table services_history but I couldn't find its model in the codebase. As far as I understanding, alembic generate revisions by checking model and database, but I am not sure why there are still revisions related to services_history even though it doesn't exist in mode.

Can someone help me to understand this? @kalbfled @jakehova

[ianperera]

I just fix migrations without removing the table, and I couldn’t find where the data was been submitted from on the services_history table.

[ianperera]

completed

[kalbfled]

@k-macmillan As you can see above revision, there is table services_history but I couldn't find its model in the codebase. As far as I understanding, alembic generate revisions by checking model and database, but I am not sure why there are still revisions related to services_history even though it doesn't exist in mode.

Can someone help me to understand this? @kalbfled @jakehova

@ianperera I'm late in seeing your question. Sorry about that. I think this is a consequence of the Versioned mixin.

[ianperera]

Just for context: flask run migrate generates the unexpected outputs. this command will never run on different environments. So this is just for local development. I made changes in the old migration script, it will never run as they are already marked as migrated. This task is just for local development which is installing the environment freshly.

[kalbfled]

The comments I've made so far apply more generally to other changes. The contents of models.py should reflect what is correct for the desired behavior of the app and what the database should codify. Changing models.py solely to eliminate unexpected migration output is not the right answer.

[ianperera]

@kalbfled I have reverted the changes based on your inputs. There are some comments in this PR that @k-macmillan asked and those changed made by you. Can you please give me some ideas about your changes? I appreciate your inputs, I learned much from your feedback.