Address redundancy re default db setup #2729
Comments
|
Newest Django-oath-toolkit 0001_initial migration contains, in part, the following model creation: Where as we have the same model creation in our storageadmin.sql.in of: With a number of follow-ups which if all interim migrations are also then applied, should result in the same state. Note also that the failure we see in: #2727 (comment)
was part of our proposed 'skipped' migrations (on new db instantiation) via the upstream squash commit: jazzband/django-oauth-toolkit@32bba99 Where "oauth2_provider/migrations/0005_auto_20170514_1141.py" contained our example failure re application.created: |
|
A pg_dump extract from our current migration of oauth2_provider via:
created by PyCharm, contained in part: With the extra fields likely down to further migrations from the above squashed 0001 migration file. |
|
A likely component here is to have non --fake-initial migrations applied, when no .initrock file exists, instead of our current sql dump use. But I'm a little concerned we don't have representative/complete 0001_initial migrations across the board. My next investigation area. It might also be nice to re-enable our admin capabilities: but his may be a step too far here, especially given the potential difficulties in migrating the last stable release similarly - which would be a priority here. |
|
From the contents of our 0001_initial migration for the default database (storageadmin) we see no:
Squashing at this point (5.0.5-0), to combine all current migrations into 0001_initial would break stable to stable updates, not ideal. We could modify our target version in this issue to that of our last stable, but again this breaks the current testing phase installs. And we only have 18 migration files for storageadmin currently. Another interim step under consideration here is to simply remove all oauth2_provider entries from our initial dump, removing them from interfering (with new db setups) with our newly instated (from upstream updates) Django-oath-toolkit. |
NotesThe result contains the following tables:
N.B. there is no django_migrations table. Pertaining to #2727
These migrations look to date back to when django-oauth-toolkit itself migrated to south: |
|
Our current "migrations" directories, including external apps: Our storageadmin.sql.in, for our default db setup, contains copies of older migrations for:
|
|
you guys opened pandora's database migration box with that innocent question of whether the database dump is still needed, it seems ... |
|
@Hooverdan96 Yes, but it would be good to have this more simplified, or at least managed by a single system: now it exists. Plus we do have a blocker here re the Django-oath-toolkit update pr linked. So we have to see to at least that for the time being. But getting there - just a little more experimentation and I hope to have a clearer picture of our options. |
|
In above comment: #2729 (comment)
Not noting in case this comes back to haunt us. I've had a look upstream and haven't seen this particular migration. |
|
The origin of our current database dump dates back to: And the following appears to be the point where our storageadmin 0001_initial was transition to native migrations. See PR: #1546 Note in: a148124#diff-ef94f8c57536aaab159776f4c01cf8f73d25130dbb7f61b6f9ad14dab6da6e82 |
With the resulting Django native migration record now established as: @FroggyFlox & @Hooverdan96 this looks promising for a move to dump all sql.in file initialisation. But as @FroggyFlox has noted in the past, we then have our smart_manager entries applied to the 'default' db of storageadmin, where-as they normally live in the smartdb database. Likely indicating we have an issue with our database router !! |
|
Our existing router is defined in settings.py as: |
|
From: https://docs.djangoproject.com/en/4.2/topics/db/multi-db/#topics-db-multi-db-routing
Our db alias use, from DATABASES in settings.py are
From: https://docs.djangoproject.com/en/4.2/topics/db/multi-db/#synchronizing-your-databases
Which is mostly what we have done to date, but now exploring routers to try and explore automating the passage of appropriate models to smart_manager, in an attempt to keep our migrations as simple as possible.
|
|
Could that be as simple as replicating the db_router.py for the storageadmin db (withe |
|
@Hooverdan96 Not entirely sure, have seen that post also. Still fathoming as it goes. Pretty sure if our existing router returns None there is a fail/fall through to the default router though. So keen to keep as simple as possible. We may have a False where there should be a None thought. [EDIT] https://dboostme.medium.com/using-django-with-multiple-databases-introduction-8f0ffb409995 |
|
I think we are just missing a 'model_name' sensitivity in our existing router. |
|
While experimenting with router configuration, it seems that if we request smart_manager model to go to smart_manager and do a generic migration we still end up with django_migrate table (within 'default'/smartmanager) showing these migrations as having been applied. This is due, I assume, the them having been considered by that database. Their associated actions are not, however, enacted upon. Strange for me, but I think it's down to Django having the ability to have the same migrations applied to multiple databases: but we want smart_manager models to only be instantiated in smart_manager. That was quite the confusion. More details to follow. I.e. we have clear upstream docs stating that https://docs.djangoproject.com/en/4.2/ref/django-admin/#migrate
So maybe we read this as the migrations for app smart_manager on the 'default' db have been run/considered:
But the router has deemed no action: I can't seem to get those migrations to go to the smart_manager db as the default command (without database specified) does not deal with anything other than the 'default' database. And these migrations are, by virtue of the in-development router changes, not targeted for this db - but are considered !! Still verifying the above with the router changes in-development.
show all migrations as applied, confirmed in django_migrations table created in storageadmin, after But: |
Also indicates having processed all migrations for all models and apps!! OK, so But on the plus side smart_manager contains the smart_manager models, but storageadmin (default) does not. Likely the in-dev model is filtering out smart_manager models for default, but not filtering out all other models intended only for default. Nearly there I think. Trying to get the router to do as much as possible for us. |
Migrates all smart_manager models (22) into postgres smartdb but showmigrations (for smart_manager), & records in django_migrations (in smartdb), all as having been processed !! None entires or completed migrations show up for default.
Migrates all Django apps (auth, django, oath2_provider) and our storageadmin models (72 in total for this db), but again showmigrations (for 'default'), & django_migrations table within storageadmin (postgresql) records all migrations as having been applied. So as-is the new router looks to do the right thing, but the end result, in each db, is that all migrations look to have been applied (irrispective of if they belong to the database being queried). So it looks like showmigrations actually shows what migrations have been processed for the target database: but only those relevant to the database in question are actually being created/applied. I think my confusion is with us previously specifying individual apps, and likely with our prior router not functioning to block smart_manager destined models from being created in 'default'. Not sure if non smart_manager models would end up in smart_manager as they did earlier here for me (but no longer) however. |
Proposed new router (linked draft-pr)Wipe both our dbs (with no current connections) via:
Looks to apply all known models/migrations with: Similarly:
Looks to apply all known models/migrations via: However as above with the default the db, smart_manager now contains only our intended 22 smart_manager models/migrations along with the django_migrations table (containg all migrations across all apps !! As having been 'processed'). So given we specified no app for either database and all models ended up where they were intended this looks to be a good start re the proposed db router. |
We currently use both an initial (years old) pg_dump, & Djangoes native migration system to setup our initial database structure. This patch proposes that we drop the initial setup via sql.in files and normalise on using only the Django migration files. The fix purpose here is to accommodate upstream squash events such as one observed in Django-oath-toolkit, which introduced a migration conflict with our existing initial/static db setup via the old storageadmin.sql.in dump file. # Includes - Revised database router to ensure the existing model/db separation regarding the smart_manager app and its use of only the dedicated smart_manager database. Along with migration awareness, so that model metadata 'app_label' informs each migration appropriately: wise to the currently specified detabase during `django-admin migrate` runs.
|
We still have the option, for say smart_manager only, to process only the relevant migrations for that db/app via: Which is much quicker and leaves us with only a record of the relevant 'processed' migrations in that database: This is the proposed approach for the new db setup mechanism (in initrock.py) that I'm working on, for both databases now. This leaves us with the generic default initial setup/migration command of;:
Which can then, auto-sort application order etc. |
- Remove old/legacy and now redundant smartdb.sql.in & storageadmin.sql.in files, and south_migrations archive directories for both databases. - Move db setup/migration code in initrock script to fstrings. - DB migration mechanisms not associated with the initial DB setup are not functionally modified, as this is outside the scope intended for this patch.
We currently use both an initial (years old) pg_dump, & Djangoes native migration system to setup our initial database structure. This patch proposes that we drop the initial setup via sql.in files and normalise on using only the Django migration files. The fix purpose here is to accommodate upstream squash events such as one observed in Django-oath-toolkit, which introduced a migration conflict with our existing initial/static db setup via the old storageadmin.sql.in dump file. # Includes - Revised database router to ensure the existing model/db separation regarding the smart_manager app and its use of only the dedicated smart_manager database. Along with migration awareness, so that model metadata 'app_label' informs each migration appropriately: wise to the currently specified detabase during `django-admin migrate` runs. - Remove old/legacy and now redundant smartdb.sql.in & storageadmin.sql.in files, and south_migrations archive directories for both databases. - Move db setup/migration code in initrock script to fstrings. - DB migration mechanisms not associated with the initial DB setup are not functionally modified, as this is outside the scope intended for this patch.
…atabase-setup Address redundancy re database setup #2729
|
Closing as: |
As highlighed by @FroggyFlox & @Hooverdan96 as part of #2723, which concerns the smart_manager database and our services setup, we may similarly have a redundancy in how we establish, after dropping storageadmin, a new setup of the default databases "storageadmin".
As Rockstor pre-dates south's integration into Django we have some layers of legacy here. We also have. as part of our DB_SETUP in initrock (for new database instances only), a straight dump of all that has gone before via:
However this dump contains many now redundant, and conflicting, migration details. For example, in this dump/restore file we have the following south migration detail as having been already applied within this sql dump:
And possibly then responsible for the indicated applied state for oauth2_provider's migrations. See the following comment for details:
#2727 (comment)
Which pertains to an older version of the initial db setup file of oauth2_provider (part of Django-oath-toolkit - an external app).
However as detailed in #2710 comment: #2710 (comment)
This initial setup/migration file for oauth2_provider has since been re-used as a squashed version now containing many of the follow-up model additions/alterations. Leading, in another part, to this issue's creation, to examine if our initial database setup, consisting of an initial (now old) db snapshot/dump file's restore, should be dropped in favour of using only migrations files to achieve the same. Especially give the dump contains models that are the responsibility of external applications. Inevitably leading to breakage/fragility when those projects modify/squash their migration files.
South migration files, and now Django's also, (over the life time of Rockstor) are 'tagged' as having been applied by their name only via a dedicated db table. This is thought to have now lead to our current inability to re-create a complete/sane oauth2_provider db setup, regarding new db setups only, our supposedly pre-applied (via dump file) migration now contains less than the post squash (into 0001_initial) migrations assume. And so some interim migrations are now missing from our external Django-oauth-toolkit library. But only on fresh db setups, as pre-existing databases prior to our ongoing work in #2727 used versions of this same library prior to this upstream squashing event.
The text was updated successfully, but these errors were encountered: