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

Deadline: Remove legacy suspend_publish attribute definition in favor of publishJobState #315

Merged
merged 8 commits into from
May 6, 2024

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Mar 28, 2024

Changelog Description

Remove legacy suspend_publish attribute definition in favor of publishJobState.

The Publish Job State became exposed some time ago through a dedicated EnumDef replacing the 'legacy' suspend_publish:
image

Additional info

n/a

Testing notes:

  1. Publishing to Deadline should be able to set publish job state and it should work:
    • Houdini
    • Maya (should not actually be altered)
    • Fusion
    • Nuke

Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

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

works in Houdini

@kalisp
Copy link
Member

kalisp commented Apr 2, 2024

Isn't working in Fusion, as instance there is just regular render. render.farm is attached only in collect_render plugin.

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

When tested in Maya 2024 and set my PublishJobState to Suspended it just suspends the actual publish job and not the render job and immedeately start to render those...

Is that correct behaviour?? As I was expecting the job simply wont start and being suspended till user resume it...

As seen here that the render job being queing and not suspended as the publish job
Screenshot 2024-04-02 160850

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 2, 2024

When tested in Maya 2024 and set my PublishJobState to Suspended it just suspends the actual publish job and not the render job and immedeately start to render those...

Is that correct behaviour?? As I was expecting the job simply wont start and being suspended till user resume it...

Yes, that is the correct behavior of that particular toggle. It's only about the Publish Job - not the Render Job state.

@kalisp
Copy link
Member

kalisp commented Apr 17, 2024

I think that in Nuke it has same issue as in Fusion, the toggle is not visible as render.farm is not there yet.

@BigRoy BigRoy self-assigned this Apr 17, 2024
…`render`, `image` and `prerender`.

This should be safe because `instance.data.get("farm")` is checked in `process()` and if not true the processing is skipped anyway - so if e.g. a render instance in Fusion is set to render local instead of on the farm the actual attribute definition does show - but the processing of the plug-in is skipped regardless.
@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 17, 2024

Ok, so I took a look at Fusion (and a bit at reading code for Nuke). I wonder how we can actually best 'target' this plug-in so that the attribute definitions show for the instances.

Fusion now has two creators, product type "image" and product type "render". To show the attribute definitions for both of those the families for this plug-in must contains those product types as families. However, "image" may conflict with product types in other hosts where we may NOT want to show it, maybe?

Anyway, I've added render and image and also prerender which I think is relevant for Nuke? The actual processing of this plug-in is skipped anyway if instance.data["farm"] is not true anyway.

Implemented with 03cfed2

Would love to know if anyone sees any issues - and/or whether it works. it worked for me in Fusion, but definitely give it a test run.

And of course, test others hosts as well that use any family render, image or prerender:

  • Fusion render product type should work
  • Fusion image product type should work
  • Blender render product type should work
  • Nuke render product type should work
  • Nuke prerender product type should work
  • Nuke image product type should work
  • Celaction render product type should continue to work
  • Harmony render product type should continue to work
  • After Effects render product type should continue to work

Pretty sure the following should be unaffected since these are not in the hosts list for this plug-in:

  • TVPaint render product type should continue to work
  • Unreal render product type should continue to work
  • Tray Publisher render product type should continue to work (Batch movies)
  • Photoshop image product type should continue to work (also for auto image)
  • Substance Painter texture set image instances should continue to work

@moonyuet
Copy link
Member

moonyuet commented Apr 18, 2024

Tested with the image instance in SP, looks good.
Tested with the render instance in Blender, also works smoothly.
image
image

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 18, 2024

Tested with the render instance in Blender, also works smoothly.

@moonyuet does the Suspend Publish ENUM toggle also show in Blender - and does it work? I assume Blender is capable of submitting to Deadline, etc? (according to your log it seems) But does it not create a publish job for it?

@kalisp
Copy link
Member

kalisp commented Apr 18, 2024

This is weird (and will be weird for all local render or image publishes).
image

I don't think this is correct solution, I would prefer to move assignment of render.farm to creation phase for Nuke and Fusion, removal render product type in DL submissions altogether.

The issue there might be if artist would switch between Local and Farm publishing, I don't think we are "repainting" toggles after that (maybe we do after Save? That would be enough, I think.)

@moonyuet
Copy link
Member

Tested with the render instance in Blender, also works smoothly.

@moonyuet does the Suspend Publish ENUM toggle also show in Blender - and does it work? I assume Blender is capable of submitting to Deadline, etc? (according to your log it seems) But does it not create a publish job for it?

it shows up the enum toggle and it can be submitted to deadline. But it does create a publish job for it.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 18, 2024

But it does create a publish job for it.

This is good - if the publish job works. The toggle doesn't decide whether it gets created or not - it only decides whether it starts "active" (which will be pending at first) or "suspended"

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 18, 2024

This is weird (and will be weird for all local render or image publishes). image

I don't think this is correct solution, I would prefer to move assignment of render.farm to creation phase for Nuke and Fusion, removal render product type in DL submissions altogether.

The issue there might be if artist would switch between Local and Farm publishing, I don't think we are "repainting" toggles after that (maybe we do after Save? That would be enough, I think.)

No - the attribute definitions are static, and only target the main product type - not something during collect stage or any "additional families" set by the creator. As such, unfortunately this is the current "best" behavior. The attribute always shows and does not disable when it becomes irrelevant due to another toggle on the instance.

In short, unfortunately that looks correct. That is if when setting render target to FARM that the publish job toggle actually does something. So for now if the enum toggle appears only on instances it CAN be relevant for under certain conditions (e.g. a specific render target) and it works to suspend the publish job or not then this PR is best as it can be.

I believe @antirotor has set up some issues tracking the Publisher improvements to be able to make toggles dependent on each other, etc. - I believe it's this one: #133

Also don't think @iLLiCiTiT sees a better way currently?

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.

I tested Nuke and Fusion, seems ok (with regard of my last comment).

@kalisp kalisp removed their assignment Apr 19, 2024
Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

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

works in Houdini

@BigRoy BigRoy assigned iLLiCiTiT and unassigned BigRoy Apr 30, 2024
@BigRoy
Copy link
Collaborator Author

BigRoy commented May 3, 2024

Ready to merge?

@kalisp kalisp merged commit 8da0f90 into ynput:develop May 6, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues and PRs coming from the community members host: Houdini host: Nuke module: Deadline size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants