Inter-App Table Dependencies and Migrations

[hr-curtissimo]

I'm following the advice of #420 - What is the best way to extend BaseUser?. I've created an app named accounts that has a Profile(Table) in it and a ForeignKey back to the BaseUser.

When I try to migrate accounts, I get an error because the piccolo_user table does not exist, even when I add the following to the APP_CONFIG of the accounts app:

migration_dependencies=["piccolo.apps.user.piccolo_app"],

Do I misunderstand the purpose of migration_dependencies?

[dantownsend]

Try also adding it to APP_REGISTRY in piccolo_conf.py:

APP_REGISTRY = AppRegistry(
    apps=[
        "my_app.piccolo_app",
        "piccolo.apps.user.piccolo_app",
    ]
)

It might be that migration_dependencies doesn't do anything if the app isn't in APP_REGISTRY.

[hr-curtissimo]

Hi, @dantownsend! Thanks for the recommendation. Unfortunately, that doesn't seem to work? Here're the files:

# piccolo_conf.py
from piccolo.engine.postgres import PostgresEngine
from piccolo.conf.apps import AppRegistry


DB = PostgresEngine(config={ ... }) # All the settings, there

APP_REGISTRY = AppRegistry(
    apps=[
        "accounts.piccolo_app",
        "piccolo.apps.user.piccolo_app",
    ]
)

# piccolo_app.py
import os

from piccolo.conf.apps import AppConfig, table_finder


CURRENT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))


APP_CONFIG = AppConfig(
    app_name="accounts",
    migrations_folder_path=os.path.join(
        CURRENT_DIRECTORY, "piccolo_migrations"
    ),
    table_classes=table_finder(
        modules=["accounts.tables"],
        exclude_imported=True,
    ),
    migration_dependencies=["piccolo.apps.user.piccolo_app"],
    commands=[],
)

And, I get the error:

... stack trace ...
asyncpg.exceptions.UndefinedTableError: relation "piccolo_user" does not exist

[dantownsend]

What happens if you run piccolo migrations forwards user?

[hr-curtissimo]

When I run the migrations for that Piccolo app specifically, then it runs the three migrations, creating and updating the piccolo_user table. After that, of course, I can run the migrations for my accounts app.

There's obviously a workaround for this. 😅

I am just curious about the migration_dependencies setting.

[sinisaos]

@hr-curtissimo I can't produce your error and everything is fine. Maybe you forgot to run

piccolo migrations forwards session_auth
piccolo migrations forwards user

and than create user with piccolo user create
I uploaded the whole project created with piccolo asgi new and added the accounts app with piccolo app new accounts
accounts.zip

Once you download the folder, delete the migrations from the home and accounts apps, add the db credentials and run your migrations like this

piccolo migrations forwards session_auth
piccolo migrations forwards user

piccolo user create

piccolo migrations new accounts --auto
piccolo migrations forwards accounts

piccolo migrations new home --auto
piccolo migrations forwards home

and you should be fine.

[dantownsend]

I had a look at the codebase. So when using piccolo migrations forwards all:

piccolo/piccolo/apps/migrations/commands/forwards.py

Lines 109 to 110 in eb6ba32

	if app_name == "all":
	sorted_app_names = BaseMigrationManager().get_sorted_app_names()

It does a very naive sort of the apps based on migration_dependencies. In other places in Piccolo, we use graphlib from the standard library for this kind of thing, so would be good to update this code to use it as well:

piccolo/piccolo/conf/apps.py

Lines 444 to 451 in eb6ba32

	def sort_app_configs(app_config_1: AppConfig, app_config_2: AppConfig):
	return (
	app_config_1 in app_config_2.migration_dependency_app_configs
	)
	sorted_configs = sorted(
	configs, key=functools.cmp_to_key(sort_app_configs)
	)

If you just pass the app name to forwards (i.e. piccolo forwards my_app), then it doesn't seem to migrate the other apps in migration_dependencies, so we might want to change this behaviour.

[sinisaos]

That's exactly the problem. According to the docs if we use migration_dependencies Piccolo should run the migration for BaseUser first and then for the accounts app (in this case) , but this is not the case and then we get an error asyncpg.exceptions.UndefinedTableError: relation "piccolo_user" does not exist
because the migration for BaseUser was never started.

[hr-curtissimo]

@dantownsend @sinisaos Is it okay if I take a try at this and submit a PR?

[dantownsend]

@hr-curtissimo That would be great, thanks!

[sinisaos]

@hr-curtissimo Yes of course, thanks for your effort.

[dantownsend]

I've opened a PR (#864) which fixes the inter-app migration sorting. Should be in the next release.