SG-43811 migrate flow write node#135
Conversation
- added .flake8 config copied from tk-core - added vim lock files to .gitignore - implemented NukeHost subclass of FlowHost NOTE: FlowWrite node will be ported in a separate ticket - instantiate NukeHost during engine set up
- add custom FlowWriteNode class to flowam module - collect renders of FlowWriteNodes in publisher - post-publish add asset id back to FlowWrite node and save file
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ticket/sg-43461/migrate-nuke-host #135 +/- ##
==================================================================
Coverage 19.89% 19.89%
==================================================================
Files 6 6
Lines 553 553
==================================================================
Hits 110 110
Misses 443 443
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…/migrate-flow-write-node
- line endings on flow_write_node.py - remove top level imports of flow modules
| HookBaseClass = sgtk.get_hook_baseclass() | ||
|
|
||
|
|
||
| class NukeFlowWritePublishPlugin(HookBaseClass): |
There was a problem hiding this comment.
Sorry I am not quite understand nuke hooks here. I am wondering why we need two different publish hooks? Are they get triggered differently?
There was a problem hiding this comment.
Yes, and my understanding is also not great. The original one only deals with Nuke "session" publishes - i.e. nuke script files. The item filter only accepts these. And since I need specific publish behaviour for the renders (which is different than scripts) - specifically a bit of post processing - the way to ensure I can "catch" those publishes is by creating a custom publisher which does the normal generic publish, but then some extra functionality after. Hope that makes sense!
There was a problem hiding this comment.
Thanks for the explanation. I am thinking should we move nuke_flow_write_publish_script.py inside nuke_publish_script.py and use context to check if we trigger flowam logic for nuke script publish workflow?
In this case, user don't need to update their config to add a new hook to be able to use flowam logic (same as collector.py/pre_publish.py/publish_file.py in native tk-multi-publsih2).
What do you think?
| ) | ||
|
|
||
| self.logger.info("Registering global callbacks for Write nodes...") | ||
| nuke.addKnobChanged(FlowWriteNode._on_update_flow_write, nodeClass="Write") |
There was a problem hiding this comment.
Do we need to remove registered callback on tear down? Would this registered callback accumulate?
There was a problem hiding this comment.
Yes this is a good idea. I haven't been doing that in the host code and probably should. Let me look into that!
|
|
||
| # Check for mandatory properties | ||
| if not write_node["asset_name"].value().strip(): | ||
| nuke.message("Asset Name must be set.") |
There was a problem hiding this comment.
I am wondering why we need two messages here. Does nuke.message display differently?
There was a problem hiding this comment.
Yes I think Nuke.message() pops up a little dialog.
carlos-villavicencio-adsk
left a comment
There was a problem hiding this comment.
I believe the code style validation check if failing.
| This modules contains the callbacks that support our specific Flow integration. | ||
| """ | ||
|
|
||
| from __future__ import annotations # needed for python 3.9 support |
There was a problem hiding this comment.
Since this code is not going to run on Houdini (because this is Nuke engine) probably we don't need this. Not sure if I'm missing something.
There was a problem hiding this comment.
Is it the case that we don't support any Nuke engines that run python 3.9? I just never know when we need this and when we don't!
There was a problem hiding this comment.
Yes, Nuke 15 is the min version supported and it comes with Python 3.10.
| from .flow_write_node import FlowWriteNode | ||
|
|
||
| # Ensure flow host exists | ||
| host = sgtk.platform.current_engine().flow_host |
There was a problem hiding this comment.
Do we want to do a core check here as well?
|
|
||
| def _in_flow_context(self) -> bool: | ||
| """Return True if in a Flow-enabled context.""" | ||
| return hasattr(self.parent.engine.context, "flow_project_id") |
There was a problem hiding this comment.
| return hasattr(self.parent.engine.context, "flow_project_id") | |
| return getattr(self.parent.engine.context, "flow_project_id", None) is not None |
Summary: Port the FlowWrite node implementation in Nuke of POC to Nuke engine, and adapt the user flow more in line with existing SG Write node.
Details: