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

No need to pass 'config' to 'host.(un)install' #505

Closed
jasperges opened this issue Jan 8, 2020 · 4 comments
Closed

No need to pass 'config' to 'host.(un)install' #505

jasperges opened this issue Jan 8, 2020 · 4 comments
Labels
document-this Needs to be documented.

Comments

@jasperges
Copy link
Contributor

When changing the Blender integration so it works with #501, I noticed that the install() and uninstall() functions still expect the config variable. If I'm not mistaken this is not needed anymore and it can be safely removed after changing this line in avalon.pipeline to host.install().

Is there a reason not to change this? I can make a pull request if needed.

@BigRoy
Copy link
Collaborator

BigRoy commented Jan 8, 2020

You are correct. Let's clean that up too. Let's try and force ourselves into there being no need for the Integrations to access the configs on install() .

Just for clarity. This is what happens on api.install() in the order as it is in the current code:

Step Description
io.install() Build api.Session from environment variables and connect to Avalon database
host.install() Trigger the Avalon Host Integration
config.host.install() The config Host Integration specific install()
register_host(host) Set the registered host
register_config(config) Set the registered config
config.install() The config global install()

Where host is the integration, e.g. maya or nuke.
Where config is the studio configuration, e.g. the default polly or your studio config.
Looking at that it still seems the order is somewhat confusing, with register_xxx in the middle there.

@tokejepsen
Copy link
Collaborator

Is the issue not with config.host.install()?

How would config do custom host installation otherwise?

@BigRoy
Copy link
Collaborator

BigRoy commented Jan 8, 2020

@tokejepsen see #501 - with that the host.install() does not trigger the config.host.install() but it's api.install() that does so. That way, the code does not need to be duplicated per host integration, plus it avoids the need for the host integrations themselves to access the studio configs on install.

Does that answer your question?

@tokejepsen
Copy link
Collaborator

Excellent!

@davidlatwe davidlatwe added the document-this Needs to be documented. label Jan 17, 2020
@BigRoy BigRoy closed this as completed in 2a68a1f Jan 24, 2020
BigRoy added a commit that referenced this issue Jan 24, 2020
Remove unneeded `config` variable from host (un)install (closes #505)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document-this Needs to be documented.
Projects
None yet
Development

No branches or pull requests

4 participants