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

Avoid requiring host in config to avoid "No module named xxx" errors #499

Closed
BigRoy opened this issue Dec 29, 2019 · 2 comments
Closed

Avoid requiring host in config to avoid "No module named xxx" errors #499

BigRoy opened this issue Dec 29, 2019 · 2 comments
Assignees

Comments

@BigRoy
Copy link
Collaborator

BigRoy commented Dec 29, 2019

Issue

Say you're a newcomer to Avalon and you'd want to try the setup in Nuke with a config that does not have {config}/nuke/__init__.py yet you will run into an ImportError error saying "No module name nuke".

The same is true for other hosts like:

  • Maya same error.
  • Houdini same error.
  • Fusion which looking at the code will actually pass but instead return the config itself as opposed to the fusion package inside of it. Which is also very wrong. It should actually set config = None.

What did you expect?

Clarify what is going on, and allow it to pass.

It should either:

  1. Show a descriptive error message on how this could be solved.
  2. Or, maybe even more preferred, allow it to actually run fine without requiring {config}/nuke module to exist. Maybe just logging a warning of something like:
    • Config "polly" has no dedicated "nuke" module. Skipping config initialization for "nuke"

I'd opt for two and make it very clear where and how one could set something like that up. Maybe even pointing to a part in the documentation on how to set up custom installations in a studio config for an integration.

Avoid code duplication

Additionally, note how the find_host_config code is duplicated among all integrations. I'd recommend instead making a single function in avalon.lib like:

# pseudocode
def find_host_config(config, host):

    config_name = config.__name__
    module_name = "%s.%s" % (config_name, host)
    try:
        config = importlib.import_module(module_name)
    except ImportError as exc:
        if str(exc) != "No module name {}".format(module_name):
            log.warning("Config has no '%s' module." % module_name)
        config = None

    return config

And then e.g. use it in the avalon.nuke.pipeline as simply as lib.find_host_config(config, "nuke").

To Reproduce

Try running e.g Nuke with the Polly config which does not have anything set up for nuke.

Additional context

This "No module" error caught my attention as someone asked me about how to get the Nuke integration up and running which he failed to try out because of a config that didn't have polly/nuke/__init__.py

@BigRoy
Copy link
Collaborator Author

BigRoy commented Dec 31, 2019

In the PR I actually proposed a different way of cleaning the duplicate code by moving the code to the global install/uninstall which makes much more sense and it would never have to be duplicated for new hosts, greatly simplifying what's required for new host integrations.

BigRoy added a commit that referenced this issue Jan 7, 2020
Patch #499: Refactor find_host_config to lib.find_module_in_config
@BigRoy
Copy link
Collaborator Author

BigRoy commented Jan 7, 2020

This is fixed with #501

Be aware that pending integrations like #466 #497 #496 #502 should update the core install and uninstall functions for the host accordingly, that is the removal of calling install and uninstall on the config's host module.

@BigRoy BigRoy closed this as completed Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant