Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

General: Default modules loaded dynamically #2923

Merged
merged 3 commits into from
Mar 28, 2022

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Mar 20, 2022

Brief description

Default modules can be loaded dynamically without adding them into DEFAULT_OPENPYPE_MODULES.

Description

Default modules had to be added in openpype/modules/base.py to be added into modules. This has changed with reverting the logic to skip known files in the folder. Also backwards compatible way of having modules in subfolder default_modules does not happend if the folder does not exist.

Changes

  • default modules are loaded dynamically so they don't have to be specified in DEFAULT_OPENPYPE_MODULES variable
  • default_modules dir is not checked for openpype modules if does not exist

Testing notes:

  1. Default modules should be loaded as they did
  2. To add new module just copy it in openpype/modules folder
    • try copy openpype/modules/example_addons/tiny_addon.py into openpype/modules/tiny_addon.py
    • the addon will be loaded
  3. Subfolder default_modules should not be checked for modules if does not exist

Should resolve issue from PR #2814

@iLLiCiTiT iLLiCiTiT self-assigned this Mar 20, 2022
@iLLiCiTiT iLLiCiTiT added the type: enhancement Enhancements to existing functionality label Mar 20, 2022
@BigRoy
Copy link
Collaborator

BigRoy commented Mar 23, 2022

I will try and check this out tomorrow.

What's the particular reason we do want the dynamic loading but do not want to use an explicit folder like default_modules for it?


Question: Could we, with adopting Avalon's "Plugin" system directly into OpenPype now opt to just use that basic behavior using a discover_modules method? Just to unify how we register "plugins"? Or what's holding us from doing so?

Modules could even be inside openpype/plugins/modules for that same consistency? Not sure if better - but it's something that crossed my mind.

@iLLiCiTiT
Copy link
Member Author

What's the particular reason we do want the dynamic loading but do not want to use an explicit folder like default_modules for it?

We've iterated over this many times and I think that final answer was that it is easier to add new modules this way (and to avoid long imports?).

Question: Could we, with adopting Avalon's "Plugin" system directly into OpenPype now opt to just use that basic behavior using a discover_modules method?

Modules and addons are one level higher. They must be available on application start because they can define settings schemas, their defaults and cli commands (and plugins). Plugins are in pipeline part. LaunchHooks will be probably added to plugins.

Copy link
Member

@kalisp kalisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works (tested in AE, Maya, Nuke)

@iLLiCiTiT iLLiCiTiT merged commit fd99c1f into develop Mar 28, 2022
@iLLiCiTiT iLLiCiTiT deleted the enhancement/dynamic_default_modules branch March 28, 2022 11:28
@BigRoy
Copy link
Collaborator

BigRoy commented Mar 30, 2022

After this PR I'm running into the issue that if there's a "leftover" folder in openpype/modules like openpype/modules/my_module that contains no files except for __pycache__ folder and __init__.pyc files then I get this error message on launch of Tray:

!!! ERR: 2022-03-30 14:29:28,111 >>> { ModulesLoader }: [  FAILED to import default module 'colorbleed'.  ]
==============================
bad magic number in 'openpype.modules.colorbleed': b'\x03\xf3\r\n'
==============================
Traceback (most recent call last):
  File "S:\openpype\OpenPype\openpype\modules\base.py", line 312, in _load_modules
    default_module = __import__(import_str, fromlist=("", ))
ImportERROR: bad magic number in 'openpype.modules.colorbleed': b'\x03\xf3\r\n'

These files remained there after checking out another branch and those files were still there from another branch that did have files in the module. Due to these remaining files (*.pyc and __pycache__) being git ignored they aren't cleaned up, etc.

@iLLiCiTiT
Copy link
Member Author

Hmm, we could force to have __init__.py in module if it's folder and check it.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 30, 2022

Hmm, we could force to have init.py in module if it's folder and check it.

Technically the module should be usable if it had only __init__.pyc instead of __init__.py if the bytecode is for the correct Python version, etc. But I guess we could that as first step for now.

@mkolar mkolar added this to the next milestone Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants