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

Automatic plugin static files collection broken after install #7709

Closed
2 of 6 tasks
wolflu05 opened this issue Jul 22, 2024 · 19 comments · Fixed by #7470 or #8503
Closed
2 of 6 tasks

Automatic plugin static files collection broken after install #7709

wolflu05 opened this issue Jul 22, 2024 · 19 comments · Fixed by #7470 or #8503
Labels
bug Identifies a bug which needs to be addressed plugin Plugin ecosystem
Milestone

Comments

@wolflu05
Copy link
Contributor

wolflu05 commented Jul 22, 2024

Please verify that this bug has NOT been raised before.

  • I checked and didn't find a similar issue

Describe the bug*

I have broken down the issues with plugin static files into multiple smaller ones:

1. Static files path (fixed with #7763)

Details

If an AppMixin plugin before had a static folder and the django app was loaded, invoke static had already collected the static files in the root of the static folder so a file in my-plugin/static/panel.js was served by /static/panel.js.
(to remove clashes between files with the same name from multiple plugins, I placed my static files in my-plugin/static/my-plugin/panel.js, so it was served by /static/my-plugin/panel.js)

Now after the mentioned PR, a file in my-plugin/static/panel.js is served by /static/plugins/my-plugin/panel.js. I do not think this change is bad, it just needs to be documented as breaking. One good think would be to provide a {% plugin_static "panel.js" %} template tag which resolves to the particular plugin static url (/static/plugins/my-plugin/panel.js) and we back port that to 0.15 which resolves to /static/panel.js

2. Automatic static file copying and migrations are not run after activating a plugin

I can reproduce this in a fresh docker setup (but not in my dev setup running locally). But no idea why only in a production docker setup. (Sometimes only very few times it actually succeeds, but then the migrations are run two times, the first time without any error, and the second they error out, because obviously the table already exists, that the migrations should create). Maybe that's some type of race condition.

  1. Goto AdminCenter > Plugins > Settings and Activate plugins that uses apps
  2. Goto AdminCenter > Plugins > Install plugin and use inventree-bulk-plugin (or inventree-kicad, but that only tests with migrations)
  3. Activate that plugin
  4. See this error in the docker logs: inventree-worker | Plugin registry has no record of plugin 'inventree-bulk-plugin'
    • See that the folder static/plugins/inventree-bulk-plugin does not exist (folder should exist)
    • See that the migrantions have not run (should have run)
  5. Run invoke update the files now exists and the migrations have run

3. When deactivating a plugin, the the static files don't get removed

When deactivating a plugin, the static files get don't removed automatically, a invoke static is needed. And even then, nested folders remain.

4. AppMixin Plugins with static files causes an error (fixed with #7763)

Details

(If 2 is solved, found with my local dev setup where I don't have the issue 2, or when running invoke static)

  1. Install a plugin with the app mixin that provides static files like in (2.1 and 2.2)
  2. Activate the plugin
  3. See the following error:
Process-c8d4a6053e4d446fb9f81077681a7693 processing blue-snake-xray-beryllium 'plugin.staticfiles.copy_plugin_static_files'
Copying static files for plugin 'inventree-bulk-plugin'
Failed 'plugin.staticfiles.copy_plugin_static_files' (blue-snake-xray-beryllium) - [Errno 66] Directory not empty: '/Users/wolflu/Development/1_GITHUB/InvenTree/dev/static/inventree-bulk-plugin' : Traceback (most recent call last):
  File "/Users/wolflu/Development/1_GITHUB/InvenTree/dev/venv/lib/python3.11/site-packages/django_q/worker.py", line 97, in worker
    res = f(*task["args"], **task["kwargs"])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wolflu/Development/1_GITHUB/InvenTree/src/backend/InvenTree/plugin/staticfiles.py", line 66, in copy_plugin_static_files
    clear_static_dir(destination_prefix)
  File "/Users/wolflu/Development/1_GITHUB/InvenTree/src/backend/InvenTree/plugin/staticfiles.py", line 31, in clear_static_dir
    staticfiles_storage.delete(d)
  File "/Users/wolflu/Development/1_GITHUB/InvenTree/dev/venv/lib/python3.11/site-packages/django/core/files/storage/filesystem.py", line 156, in delete
    os.rmdir(name)
OSError: [Errno 66] Directory not empty: '/Users/wolflu/Development/1_GITHUB/InvenTree/dev/static/inventree-bulk-plugin'

5. Property bug (can only sometimes reproduce) => cannot reproduce anymore

When freshly installing a plugin (freshly, not previously installed), and then activating the plugin without restarting the server it can happen that it looks like this:
image

Error

Traceback (most recent call last):

File "/root/.local/lib/python3.11/site-packages/rest_framework/views.py", line 506, in dispatch
response = handler(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

File "/home/inventree/src/backend/InvenTree/plugin/api.py", line 341, in get
settings_definition=settings, plugin=plugin.plugin_config()
^^^^^^^^^^^^^^^^^^^^^^

TypeError: MetaBase.plugin_config() missing 1 required positional argument: 'self'

Deployment Method

  • Docker
  • Package
  • Bare metal
  • Other - added info in Steps to Reproduce

Version Information

# Version Information:
InvenTree-Version: 0.17.0 dev
Django Version: 4.2.15
Commit Hash: cc45c23
Commit Date: 2024-08-30

Database: postgresql
Debug-Mode: False
Deployed using Docker: True
Platform: Linux-6.6.31-linuxkit-x86_64-with
Installer: DOC

Active plugins: [{'name': 'InvenTreeBarcode', 'slug': 'inventreebarcode', 'version': '2.1.0'}, {'name': 'InvenTreeCoreNotificationsPlugin', 'slug': 'inventreecorenotificationsplugin', 'version': '1.0.0'}, {'name': 'InvenTreeCurrencyExchange', 'slug': 'inventreecurrencyexchange', 'version': '1.0.0'}, {'name': 'InvenTreeLabel', 'slug': 'inventreelabel', 'version': '1.1.0'}, {'name': 'InvenTreeLabelMachine', 'slug': 'inventreelabelmachine', 'version': '1.0.0'}, {'name': 'InvenTreeLabelSheet', 'slug': 'inventreelabelsheet', 'version': '1.0.0'}, {'name': 'DigiKeyPlugin', 'slug': 'digikeyplugin', 'version': '1.0.0'}, {'name': 'LCSCPlugin', 'slug': 'lcscplugin', 'version': '1.0.0'}, {'name': 'MouserPlugin', 'slug': 'mouserplugin', 'version': '1.0.0'}, {'name': 'TMEPlugin', 'slug': 'tmeplugin', 'version': '1.0.0'}]

Please verify if you can reproduce this bug on the demo site.

  • I can reproduce this bug on the demo site.

Relevant log output

No response

@wolflu05 wolflu05 added bug Identifies a bug which needs to be addressed question This is a question triage:not-checked Item was not checked by the core team labels Jul 22, 2024
@SchrodingersGat SchrodingersGat removed the triage:not-checked Item was not checked by the core team label Jul 23, 2024
@SchrodingersGat
Copy link
Member

@luwol03 a few different things to address here!

Static File Pathing

  • You say that the path for serving static files is different? This was not intentional. Can you provide examples of the differences?

Static File Copying Does Not Work

  • Let's work out why!

@wolflu05
Copy link
Contributor Author

wolflu05 commented Jul 23, 2024

Good idea to break it into different issues:

EDIT: I have added this list to the initial comment now

@matmair matmair added this to the 0.16.0 milestone Jul 24, 2024
@matmair matmair removed the question This is a question label Jul 24, 2024
@matmair
Copy link
Member

matmair commented Jul 24, 2024

I think the migration part of 2 and 5 should be split out as individual issues as they do not seem connected with static content "stuff".

@matmair matmair added the plugin Plugin ecosystem label Jul 24, 2024
@wolflu05
Copy link
Contributor Author

wolflu05 commented Jul 24, 2024

I agree with point 5, but when looking deeper, issue 2 targets exactly the code added in the static files for plugins pr. It works in dev, but not, (reproducable) in a fresh docker prod setup. And I think if the static file not being collected automatically issue is solved, the task that is offloaded to run the migrations will be fixed too.

Can you reproduce that too in a fresh docker prod setup?

@SchrodingersGat
Copy link
Member

@wolflu05 I think this has been addressed now, please let me know if this is incorrect...

@wolflu05
Copy link
Contributor Author

wolflu05 commented Aug 5, 2024

Only 1 and 4 have been fixed be me in one of my recent prs. 2,3 and 5 have been still not fixed from here: #7709 (comment)

@SchrodingersGat
Copy link
Member

SchrodingersGat commented Aug 5, 2024

@wolflu05 good point. If you have some time to look into those? Otherwise I will try to get to it when I can :)

@wolflu05
Copy link
Contributor Author

wolflu05 commented Aug 5, 2024

Sorry, but unfortunately I won't have any time the next weeks.

@SchrodingersGat SchrodingersGat mentioned this issue Aug 14, 2024
6 tasks
@SchrodingersGat SchrodingersGat modified the milestones: 0.16.0, 0.17.0 Aug 14, 2024
@matmair
Copy link
Member

matmair commented Aug 30, 2024

Is this issue still happening?

@wolflu05
Copy link
Contributor Author

Is this issue still happening?

Yes, I have updated the initial comment now with the list where I have broken down the issue into smaller ones (previously this was only a comment in this discussion, so that this doesn't get lost). Issue 2 and 3 are still not solved. And issue 5 is not reproducible. I have tested with the current latest docker image (updated the version information accordingly).

@wolflu05
Copy link
Contributor Author

wolflu05 commented Sep 19, 2024

I have tested again with ec2051d, and 2,3 and 5 is still not resolved, so I'm reopening it.

Now removing the files after disabling the plugin is implemented (3), but the same error as (2) occurs when running without a restart. And I can actually reproduce 5 now again, that's a very strange one.

@wolflu05 wolflu05 reopened this Sep 19, 2024
@SchrodingersGat
Copy link
Member

@wolflu05 OK, let's work it out!

@SchrodingersGat
Copy link
Member

I have tested again with ec2051d

@wolflu05 can you please describe further your steps to reproduce? This has been working well as far as I can tell, perhaps there are some differences between our setups

@wolflu05
Copy link
Contributor Author

I have been using the default docker setup like described in the docs to reproduce it.

@SchrodingersGat
Copy link
Member

Ok, I have worked out a fix for 2) and 3) - incoming

@SchrodingersGat
Copy link
Member

@wolflu05 checkout #8502 and LMK what you think

@wolflu05
Copy link
Contributor Author

@SchrodingersGat ok, I tested it now, it seems to be still not working correctly. I setup the system by using the default prod docker setup, just change the image to latest in the .env file. Then activated all plugin support (app mixin, urls, ...). Next installed inventree-bulk-plugin. And the I got the message no plugin with key ... found. I checked the db (if migrations from my AppMixin have been run) and static files, nothing there. I let the PC sit because I had something else todo, and when I came back, it seems like a job has picked up the migration work at least. For static files, I had to manually do a restart of the server+worker container.

But the static files removal works now pretty good after uninstalling the plugin (3). And I was not able to reproduce the property bug (5) anymore.

@wolflu05 wolflu05 reopened this Nov 25, 2024
@SchrodingersGat
Copy link
Member

@wolflu05 can you please provide aa detailed set of logs and screenshots to show exactly the problem?

@wolflu05
Copy link
Contributor Author

I have tried to reproduce this now a couple of times, but I'm not able. Sorry for the confusion. It seems to work now. Thank you very much your effort finding and fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identifies a bug which needs to be addressed plugin Plugin ecosystem
Projects
None yet
3 participants