Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Remove multi-context restrictions for publishing image sequences on farm #6088

Closed

Conversation

antirotor
Copy link
Member

Changelog Description

Removing assert that context data must be same as instance data will allow publishing image data to different context in one go.
That sanity check was there because we didn't support publishing to different context before (like multishot workflow), but this should be possible now.

Additional info

More work on different DCCs needs to be done to seamlessly support multishot workflow - like Maya restrict only one rendering instance, Nuke enforce unique instance name (so you can't publish renderMain to multiple shots), etc.

Testing notes:

  1. Open Nuke
  2. Create 3 write instances (you need to change variant name to make them unique)
  3. Direct them in the Publisher to different shots
  4. Render and publish on farm.

@antirotor antirotor added type: enhancement Enhancements to existing functionality module: Deadline AWS Deadline related features module: RoyalRender RoyalRender related features target: OpenPype target: AYON labels Dec 22, 2023
@antirotor antirotor self-assigned this Dec 22, 2023
@ynbot ynbot added size/XS Denotes a PR changes 0-99 lines, ignoring general files and removed module: Deadline AWS Deadline related features module: RoyalRender RoyalRender related features labels Dec 22, 2023
@BigRoy
Copy link
Collaborator

BigRoy commented Dec 22, 2023

like Maya restrict only one rendering instance,

I believe in Maya each renderlayer does get its own context settings and you can override them to point to different contexts (e.g. another folder) but like what you describe for Nuke is also an issue for Maya. The renderlayer name describes the variant and must be unique - as such you cannot render the same variant (and subset) to both your current scene and one renderlayer with the same subset for another renderlayer in the same scene.

Copy link
Member

@iLLiCiTiT iLLiCiTiT left a comment

Choose a reason for hiding this comment

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

LGTM

@antirotor
Copy link
Member Author

I believe in Maya each renderlayer does get its own context settings

yes, you are right, now you can set the context on individual render layer so it's basically the same issue as with Nuke

Copy link
Member

@moonyuet moonyuet left a comment

Choose a reason for hiding this comment

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

I publish the write instances with different variant to farm successfully. These are all rendered successfully without any issue.
image

Copy link
Member

@jakubjezek001 jakubjezek001 left a comment

Choose a reason for hiding this comment

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

looking good

@mkolar
Copy link
Member

mkolar commented Feb 9, 2024

@antirotor let's get this ported to AYON and merged please

Comment on lines -78 to -94
# set context by first json file
ctx = self._context.data

ctx["asset"] = ctx.get("asset") or data.get("asset")
ctx["intent"] = ctx.get("intent") or data.get("intent")
ctx["comment"] = ctx.get("comment") or data.get("comment")
ctx["user"] = ctx.get("user") or data.get("user")
ctx["version"] = ctx.get("version") or data.get("version")

# basic sanity check to see if we are working in same context
# if some other json file has different context, bail out.
ctx_err = "inconsistent contexts in json files - %s"
assert ctx.get("asset") == data.get("asset"), ctx_err % "asset"
assert ctx.get("intent") == data.get("intent"), ctx_err % "intent"
assert ctx.get("comment") == data.get("comment"), ctx_err % "comment"
assert ctx.get("user") == data.get("user"), ctx_err % "user"
assert ctx.get("version") == data.get("version"), ctx_err % "version"
Copy link
Collaborator

@BigRoy BigRoy Feb 13, 2024

Choose a reason for hiding this comment

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

Technically - when is this is valid? If I have two .json files that specify a different running context to publish in this may be very dangerous because now suddenly that context isn't actually used for ONE of the json files?

I think, yes we should not assert that instance context must MATCH the global context - but if we were to load multiple JSON files at once we should still assert the global context of both JSON files is shared between them.

Copy link
Member Author

Choose a reason for hiding this comment

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

you're right, but I don't think that we are actually using multiple file anywhere. But if that's so, we should probably remove support for multiple json files.

@tokejepsen tokejepsen marked this pull request as draft February 20, 2024 08:32
@tokejepsen
Copy link
Member

@antirotor could we address @BigRoy comment before testing again?

@antirotor
Copy link
Member Author

I am closing it in favor of ynput/ayon-core#138

@antirotor antirotor closed this Feb 29, 2024
@ynbot ynbot added this to the next-patch milestone Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
port to AYON size/XS Denotes a PR changes 0-99 lines, ignoring general files target: AYON target: OpenPype type: enhancement Enhancements to existing functionality
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants