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

Creating a new tenant will end up with bulkrax migrations being run during the next db:migrate, resulting in odd behavior. #2387

Open
gleithner opened this issue Nov 21, 2024 · 1 comment

Comments

@gleithner
Copy link

Version 6.0.0.920da19baa

When you create a tenant, the Apartment gem loads the db/schema.rb but for some reason only the Hyku migration versions are added to the schema_migrations table for the new tenant. The next time db:migrate is run, the bulkrax migrations will attempt to run but because the schema.rb already contains these migrations, most of them will not affect the schema. But there are cases in which they are actually applied, for instance, when columns are added but then are deleted by later migrations so the result ends up being the same as the schema.rb.

However, I've noticed one case where the migrations being run against the tenant causes an unwelcome, though probably harmless, result:
bulkrax migration 20191204191623 adds columns processed_children and failed_children to bulkrax_importer_runs, and then 20211203195233 renames these columns to processed_relationships and failed_relationships, respectively. So, in the new tenant this results in bulkrax_importer_runs containing all four columns due to the fact that processed_relationships and failed_relationships already exist in the schema.

As to whether this should be fixed, or how it should be fixed, or whether it's an Apartment bug, I'm not sure. But there might be a scenario in which a reapplied migration could cause a problem beyond just extra columns.

As an aside, as far as I can tell from looking at the code Apartment only loads the schema and does not attempt to run migrations when creating a new tenant. Although this doesn't fix the original issue, I've experimented with forcing the migrations to run after schema creation by kicking off Apartment::Migrator from CreateAccount. It seems to work if I do Rails.application.load_tasks first, which may give a clue to why the bulkrax migrations are ignored in the first place when the schema is loaded, so I guess this might be a result of the way dependencies exist in Rails?

Steps to reproduce the behavior

  1. db:create && db:schema:load && db:seed
  2. In the DB, switch to the new public schema, and select count(version) from schema_migrations - 186 rows - includes all the hyku(144) and bulkrax migrations(42)
  3. Create a new tenant.
  4. In the DB, switch to the new tenant schema, and select count(version) from schema_migrations - 145 rows - hyku migrations
  5. \d bulkrax_importer_runs and observe that processed_relationships and failed_relationships columns do exist, but not processed_children nor failed_children
  6. Run db:migrate, and watch the bulkrax migrations attempt to apply to the tenant.
  7. In the DB, switch to the new tenant schema, Check count in schema_migrations and see 186 rows.
  8. \d bulkrax_importer_runs and observe all four columns now exist.
@orangewolf
Copy link
Member

Thank you for this! We've known for a while that there was an issue with Bulkrax migrations applying twice and all Bulkrax migrations are supposed to be written to prevent duplicates from being created. However removing the double run of them would certainly help the situation.

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