Project

Profile

Help

Issue #3012

db-reset script fails to migrate

Added by amacdona@redhat.com about 2 years ago. Updated 6 months ago.

Status:
MODIFIED
Priority:
Normal
Category:
-
Sprint/Milestone:
Start date:
Due date:
Severity:
2. Medium
Version:
Platform Release:
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Dev Environment
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 25

Description


Operations to perform:
  Apply all migrations: auth
Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0001_initial... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying auth.0007_alter_validators_add_error_messages... OK
  Applying auth.0008_alter_user_username_max_length... OK
[jortel@f25a app]$ python3 manage.py migrate --noinput
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, sessions
Running migrations:
  Applying admin.0001_initial...Traceback (most recent call last):
  File "/usr/lib/python3.5/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
psycopg2.ProgrammingError: relation "pulp_app_user" does not exist
remove the migrations directory for each app

Workaround:

First remove the migrations at `pulp_file/app/migrations/` and `platform/pulcore/app/migrations/`
Then create new migrations explicitly and migrate (order is important).

python manage.py reset_db
python manage.py makemigrations pulp_file
python manage.py makemigrations pulp_app
python manage.py migrate auth
python manage.py migrate pulp_file
python manage.py migrate

Associated revisions

Revision 257274ea View on GitHub
Added by amacdona@redhat.com about 2 years ago

Remove the use of db-reset script

Replace use of db-reset with pulp-manager, which will create migrations
for each plugin and migrate.

re: #3012

Revision 05b3ae3d View on GitHub
Added by amacdona@redhat.com about 2 years ago

Remove db-reset script

Where it is used in this repo, it is replaced with pulp-manager use
directly.

re: #3012

Revision 05b3ae3d View on GitHub
Added by amacdona@redhat.com about 2 years ago

Remove db-reset script

Where it is used in this repo, it is replaced with pulp-manager use
directly.

re: #3012

Revision 05b3ae3d View on GitHub
Added by amacdona@redhat.com about 2 years ago

Remove db-reset script

Where it is used in this repo, it is replaced with pulp-manager use
directly.

re: #3012

History

#2 Updated by amacdona@redhat.com about 2 years ago

  • Status changed from ASSIGNED to POST

#3 Updated by ttereshc about 2 years ago

  • Triaged changed from No to Yes

#4 Updated by amacdona@redhat.com about 2 years ago

(pulp) [vagrant@pulp3 pulp]$ pip freeze | grep Django
Django==1.11.5

(pulp) [vagrant@pulp3 pulp]$ pip freeze | grep django
django-crispy-forms==1.6.1
django-extensions==1.9.0
django-filter==1.0.4
djangorestframework==3.6.4

My suspicion is that this is related to the Django version, I did not experience this error on 1.8.

To move forward, we have a few options:
1) We can inform our infrastructure (db-reset.sh and similar) about the each app (pulp_app and each plugin) and makemigrations explicitly in pclean, Ansible, etc This isn't a great option because we don't want to hardcode the existence of plugins.
I've done this here: https://github.com/pulp/pulp/pull/3146

2) We can document the need for plugin migrations to be made before migrations are run. This would be correct, but inconvenient. We would not have a migrated database from running Ansible playbooks. We would lose `pclean`.

3) We could commit the migrations. 1) and 2) go away, but we have new questions. Do we add migrations on top of the initial one, or do we recreate the initial migrations each model change? If someone commits a model change but forgets to commit the corresponding migration, it could be difficult to debug errors. One safeguard would be to add `makemigrations` to `pclean`. Even if the developer missed it in their commit, someone pulling in their changes would run pclean if things weren't working. When they do that, their environment would be fixed, and they would have the new migration in their git tracker.

#5 Updated by daviddavis about 2 years ago

The django documentation recommends adding migrations to source control [0] so I think we ought to do option 3. Maybe not for the alpha but eventually.

Would recreating migrations on each model change be possible with upgrades? If a user upgrades from say 3.0 to 3.1, how would that work if we've recreated a migration they've already run?

Regarding devs forgetting to update migrations, I don't think that'll be a big issue. It hasn't been for the many years I've work on Foreman. Things tend not to break very visibly if databases schemas aren't correct.

[0] https://docs.djangoproject.com/en/1.11/topics/migrations/#module-django.db.migrations

#6 Updated by amacdona@redhat.com about 2 years ago

wrote:

The django documentation recommends adding migrations to source control [0] so I think we ought to do option 3. Maybe not for the alpha but eventually.

Would recreating migrations on each model change be possible with upgrades? If a user upgrades from say 3.0 to 3.1, how would that work if we've recreated a migration they've already run?

AFAIK, we are all in agreement that the migrations will be in source control before the GA. The disagreement is whether to commit them now or later.

#7 Updated by amacdona@redhat.com about 2 years ago

Option 4:

Add `makemigrations pulp_app` to Ansible before reset_db role is used. Add `makemigrations plugin_lablel` to plugin installation instructions and Ansible plugin installer.

I've tested this workflow:
0: Flush db, delete the migrations dir in all apps. Uninstall plugins.
1. Migrate core alone

pulp-manager makemigrations pulp_app # We have to explicitly state the label if migrations don't already exist
pulp-manager migrate auth
pulp-manager migrate

2. Install plugin
pip install -e ~/devel/pulp_file/
pulp-manager makemigrations pulp_file # Again, we have to explicitly give the label since no migrations exist
pulp-manager migrate # We don't need to migrate auth explicitly except for the first time

This pattern will fail when core migrations are run for the first time if pulp_file is installed but its migrations are not made .
This seems easy enough to avoid. For a typical db reset/flush, you won't delete the migrations. If you do, you'll need to recreate them before you reset the db with pclean.

#8 Updated by bmbouter about 2 years ago

+1 from me for option 4. I prefer not committing the migrations for now because when we make model changes we will need to remember to commit the migrations. That is easy, but if we forget, it may take a bit more time to track down than option 4. I think we should commit our migrations with beta1.

The Ansible installer role for any given plugin sounds fine, but I think the "pip via manual checkout" and "pip via pypi" methods are ok for alpha1. If the playbook only installs the plugin then its just saving a few commands its probably not worth it. If the plugin's Ansible playbook uses the pulpcore installation role and then installs the plugin's role I think it would be totally worth it.

#9 Updated by amacdona@redhat.com about 2 years ago

This discussion is complex because we have to consider core and multiple plugins, and we also have to consider all supported installation methods.

We have to consider how we will install plugins from Ansible, because we want a dev environment to work out of the box (which requires at least 1, plugin). We can commit core migrations today if we want, but we cant commit migrations for pulp_rpm, so we need to handle the general case.

I think the best plan is to modify the `plugins` role to a `plugin` role, which is general and installs 1 plugin based on parameters. This prevents us from hardcoding in the roles, and moves plugin info to the playbook, which is the right place for developers to make choices for their local env.

#10 Updated by amacdona@redhat.com about 2 years ago

Earlier PRs have been closed.

https://github.com/pulp/pulp/pull/3163
https://github.com/pulp/devel/pull/91

These implement Option 4, which allows Ansible to install (and makemigrations) for any plugin specified in the playbook. This will allow easy installation of plugins with and without migrations. When we do commit the migrations for core, we can remove that part from the Ansible code while still making migrations for plugins that don't have them.

For clarity, I'll sum up the install workflow.

1. Install pulp. After the install, but before plugins are installed, makemigrations and optionally migrate.
2. Install each plugin, makemigrations for it.
3. Migrate (you still have to migrate auth first).

#11 Updated by amacdona@redhat.com about 2 years ago

  • Status changed from POST to MODIFIED

#12 Updated by pcreech about 2 years ago

  • Tags Pulp 3 added

#13 Updated by bmbouter over 1 year ago

  • Sprint set to Sprint 25

#14 Updated by bmbouter over 1 year ago

  • Sprint/Milestone deleted (44)

#15 Updated by daviddavis 6 months ago

  • Sprint/Milestone set to 3.0

#16 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3)

Please register to edit this issue

Also available in: Atom PDF