inventree / InvenTree

Open Source Inventory Management System
https://docs.inventree.org
MIT License
4.19k stars 754 forks source link

Custom apps (`AppMixin` plugins) get not loaded for `inv migrate` #3549

Closed wolflu05 closed 2 years ago

wolflu05 commented 2 years ago

Seems like inv server discovers that there is an unapplied migration but inv migrate don't.

```bash vscode ➜ /workspaces/InvenTree (master ✗) $ inv server Waiting for database... Database connection sucessful! Watching for file changes with StatReloader Watching for file changes with StatReloader Performing system checks... System check identified no issues (1 silenced). You have 1 unapplied migration(s). Your project may not work properly until you apply the migrations for app(s): inventree_bulk. Run 'python manage.py migrate' to apply them. August 14, 2022 - 07:40:01 Django version 3.2.15, using settings 'InvenTree.settings' Starting development server at http://127.0.0.1:8000/ Quit the server with CONTROL-C. ^Cvscode ➜ /workspaces/InvenTree (master ✗) $ inv migrate Running InvenTree database migrations... ======================================== No changes detected Operations to perform: Apply all migrations: account, admin, auth, authtoken, build, common, company, contenttypes, django_q, error_report, exchange, label, order, otp_static, otp_totp, part, plugin, report, sites, socialaccount, stock, user_sessions, users Running migrations: No migrations to apply. Operations to perform: Synchronize unmigrated apps: InvenTree, allauth, allauth_2fa, corsheaders, crispy_forms, django_cleanup, django_filters, django_otp, djmoney, formtools, import_export, maintenance_mode, markdownify, messages, mptt, rest_framework, sslserver, staticfiles Apply all migrations: account, admin, auth, authtoken, build, common, company, contenttypes, django_q, error_report, exchange, label, order, otp_static, otp_totp, part, plugin, report, sites, socialaccount, stock, user_sessions, users Synchronizing apps without migrations: Creating tables... Running deferred SQL... Running migrations: No migrations to apply. System check identified no issues (1 silenced). ======================================== InvenTree database migrations completed! Rebuilding Part objects Rebuilding PartCategory objects Rebuilding StockItem objects Rebuilding StockLocation objects Rebuilding Build objects vscode ➜ /workspaces/InvenTree (master ✗) $ ```

Originally posted by @wolflu05 in https://github.com/inventree/InvenTree/discussions/3523#discussioncomment-3392031

Sorry @matmair, that I was asking in #3534.

wolflu05 commented 2 years ago

@SchrodingersGat I hope this issues is not sunk in all other issues, but this needs some labels.

matmair commented 2 years ago

@wolflu05 just a short reminder that we are doing this in our free time and do not have an SLA. More than 24h to react would be nice.

wolflu05 commented 2 years ago

I think I found the culprit after lots of debugging with the devcontainer and isolated two problematic points (one of this is related with the registry and one only to the devcontainer).

  1. Lets start with the easier one related to the devcontainer. In devcontainer.json is the INVENTREE_PLUGINS_ENABLED env missing, so plugins are not loaded generally, also for inv server:

    diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
    index 3d77ab14a..3300518b5 100644
    --- a/.devcontainer/devcontainer.json
    +++ b/.devcontainer/devcontainer.json
    @@ -73,6 +77,7 @@
        "INVENTREE_STATIC_ROOT": "/workspaces/InvenTree/dev/static",
        "INVENTREE_CONFIG_FILE": "/workspaces/InvenTree/dev/config.yaml",
        "INVENTREE_SECRET_KEY_FILE": "/workspaces/InvenTree/dev/secret_key.txt",
    +    "INVENTREE_PLUGINS_ENABLED": "True",
        "INVENTREE_PLUGIN_DIR": "/workspaces/InvenTree/dev/plugins",
        "INVENTREE_PLUGIN_FILE": "/workspaces/InvenTree/dev/plugins.txt",
  2. There is a canAppAccessDatabase(...) check in https://github.com/inventree/InvenTree/blob/6eddcd3c23b1ee64dd49c48da97ba8d6a3117a50/InvenTree/plugin/apps.py#L30-L33 which also prevents loading the plugins. Looking deeper in the implementation of the canAppAccessDatabase function we can see, that it returns false if Django manage.py was executed with specific commands. https://github.com/inventree/InvenTree/blob/0c97a50e47728466089c0463d5518bf267177de6/InvenTree/InvenTree/ready.py#L46-L48 and here excluded_commands contains migrate which causes the plugins to not load. Changing the following enables loading the plugins for migrations.

    diff --git a/InvenTree/InvenTree/ready.py b/InvenTree/InvenTree/ready.py
    index f7a319a92..7bfa6f782 100644
    --- a/InvenTree/InvenTree/ready.py
    +++ b/InvenTree/InvenTree/ready.py
    @@ -26,7 +26,7 @@ def canAppAccessDatabase(allow_test=False):
            'loaddata',
            'dumpdata',
            'makemigrations',
    -        'migrate',
    +        # 'migrate',
            'check',
            'shell',
            'createsuperuser',

    So I think https://github.com/inventree/InvenTree/commit/9446702d78bc34900ebb338f34f79c032467463a is a regression and should be handled more cleanly, maybe only loading AppMixin plugins for migrations?

SchrodingersGat commented 2 years ago

@wolflu05 good investigation here! @matmair does his suggestion make sense do you think?

matmair commented 2 years ago

Seems like a reasonable conclusion @wolflu05 . I am currently not using devcontainers so I can not speak with confidence to 1) but 2) would make sense.

I am using a custom loading/state management setup in my deployments so this error never came up on my side. I will try to reproduce this once I am caught up with the messages from the last few days but looks to be an easy fix, a PR should be forthcoming in the next days.

wolflu05 commented 2 years ago

@matmair I don't want to push this, just wanted to ask what the current state of fixing this issue is. I saw there is a commit: https://github.com/matmair/InvenTree/commit/52af1966941ce564bae485988c40b8161d6f201e , but no corresponding PR. Is there anything I missed?

matmair commented 2 years ago

@wolflu05 I had to prioritise other features for my fork/work; I do not know when I will find time to upstream my changes

wolflu05 commented 2 years ago

I don't want to push someone, but I want to ask friendly, if there is any progress on this.