General: Host implementation defined with class

[iLLiCiTiT]

Brief description

Initial PR to define host implementation using class instead of using functions from module.

Description

Primary goal is to somehow define what must, should and can be implemented in host to enable certain functionality (public interface for tools and global plugins). Some functionality is not directly defined in base class but in interfaces (e.g. workfiles or load workflow). The reason is that not all hosts support these features but they should be defined somewhere. Maya was used for an "example" usage of class just to match minimum requirements. It does not mean that all functions should be moved to the class, some functions can be used without installed host (e.g. during farm publishing) but the class should provide the functionality.

Secondary goal is for further future features e.g. to implement common logic in the base class and move there some host related logic which lives in global scope right now, which is not possible ATM. That would break backwards compatibility so it must wait until all hosts use the class.

Additional info

Now is the time for discussion and criticism.

TODO

• add documentation after discussion

Testing notes:

1. Run Maya
2. Everything should work as expected.

[ynbot]

Task linked: OP-3315 Host implementation defined with class

[icyvapor]

Finding it a bit odd to include "Implementation" into the class name. It's an programming concept.. and as the first line of the class comment states:
"Host is pipeline implementation of DCC application". So it feels a bit redundant.
Any issue with simply calling it "Host" ?

[iLLiCiTiT]

I'm not againts but Host is too short (for my taste), maybe BaseHost or HostBase?

[BigRoy]

Any issue with simply calling it "Host" ?

I personally felt the same as @icyvapor - I'd prefer shorter if possible.
Same for MayaHostImplementation - I'm not sure why it couldn't be called MayaHost.
From a class naming convention I'd understand MayaHost better than MayaHostImplementation. The latter almost sounds like there's still another MayaHost class somewhere in the code base.

[iLLiCiTiT]

Changed to HostBase and MayaHost, is it ok? Any other comments?

[mkolar]

The only problem with host as a word, that it's super hard to find it in the code base when you need to trace it's usages. HostBase and MayaHost is pretty good and specific enough not to have millions of false positives when searching through codebase

[BigRoy]

Would AsbtractHost make sense too? I'm also fine with HostBase by the way.

[iLLiCiTiT]

Only abstract part of the class is name property. In future is planned to have more pre-implemented functionality thus I'm more in HostBase.

[icyvapor]

Sorry guys, soon after the comment I was put out of commission for the rest of the week by some health issue. Thanks for addressing the comment.

[antirotor]

I have one request: at least for the new files, lets adhere more to some docstring and code-style culture. For example docstrings - they should use napoleon format (there are now parts in markdown that will never render correctly in sphinx when we start generating documentation). Here is some simple guide to it.

Another thing are type hints. For python 2 compatibility we should add typing backport to python 2 hosts but I think we should do type hinting at least on the docstring level.

[iLLiCiTiT]

Some notes

Method names

Changed names of some required methods:

• ls -> get_referenced_containers get_containers
• open_file -> open_workfile
• file_extensions -> get_workfile_extensions
• save_file -> save_current_workfile save_workfile
• open_file -> open_workfile
• current_file -> get_current_workfile
• has_unsaved_changes -> workfile_has_unsaved_changes
  These methods are available in default interface to not break backwards compatibility all over the place.

Type hints

Type hints are an issue because they require imported classes at the top. To be able import them in Python 2 we would have to add typing module in Python 2 vendors, but if a DCC already has one with different name (like typing27) it cause infinite loops in type checks which is case for some DCCs with specific versions of shiboken.

Documentation

Added some documentation related to host implementation. I'm not sure what approach should be used, so I've tried to explain it a way I would explain it to someone on community. Any comments regarding style, naming, grammar, ... are welcomed.

[BigRoy]

ls -> get_referenced_containers

Why not get_containers instead of get_referenced_containers?

save_file -> save_current_workfile

I'd say save_workfile makes just as much sense, no? Especially because the function allows to 'save as' a different name.

[mkolar]

This is good to merge now. We can further improve of course, but the sooner we transition to the approach the easier it will be to move on with any global improvements.

[antirotor]

not sure about the log leve here, nor the text. The is mainly interesting for developer so it should be debug. Or if not, it should be more user friendly than _on_scene_save.. that will remind artist of an ascii art.

[iLLiCiTiT]

All of them were there before this PR. Irrelevant from point of this PR.

[mkolar]

I agree. As much as there refactoring PRs uncover a lot of tech debt, it should be logged into Issues and handled separately. To keep focus on what these PRs are doing.

this is for another PR