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

Refactor Deadline Submission JobInfo #49

Merged
merged 164 commits into from
Nov 22, 2024

Conversation

kalisp
Copy link
Member

@kalisp kalisp commented Oct 18, 2024

Changelog Description

This PR tries to make DL submissions easier, mostly handling of JobInfo segment. Collection of values is moved out of submission phase much earlier where values could be filled from default values in Settings or overriden by selected list of field by artist in Publisher UI.

Move to universal collector should help with potential validation of filled values.
Profiles in Settings should allow more control of values (like priority, limit_groups) based on task_types or hosts etc.

jobinfo_settings
jobinfo_publisher

Additional info

Should be used with ynput/ayon-core#958 !!

All DCC need to be tested:

  • AfterEffects
  • Blender
  • CelAction
  • Fusion
  • Harmony
  • Houdini
  • Max
  • Maya
  • Nuke
  • Unreal

Testing notes:

Should be used with ynput/ayon-core#958 !!

  1. do regular DL publishing
  2. test that nothing breaks
  3. update ayon+settings://deadline/publish/CollectJobInfo with new defaults and check if they propagete to Publisher
  4. update values in Publisher if they propagate to DL

These settings should be base of generic JobInfo values for Deadline submission. They should contain variables previously contained in Submit* Settings.
Some of them should be exposed to Publisher UI as artist overrides.
It was decided that dataclasses should be used instead of attrs. This moves DeadlineJobInfo which is full mapping of JobInfo from abstract submitters to collectors to limit need of new class and necessary remapping later.
Added new argument for old get_job_info (which should be probabaly renamed) to pass base of prepared object to be enhanced with DCC specific fields
Empty strings overrides None defaults which might cause issue (it definitely does for job_delay).
'deadline' dictionary wasnt used at all, it contained large DeadlineJobInfo which just enlarged metadata json unnecessary.
It is handling EnvironmentKey* type of fields
@kalisp kalisp self-assigned this Oct 18, 2024
Copy link
Contributor

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

I like where this is going! :)

client/ayon_deadline/lib.py Outdated Show resolved Hide resolved
client/ayon_deadline/abstract_submit_deadline.py Outdated Show resolved Hide resolved
Comment on lines 62 to 80
chunk_size: int = SettingsField(999, title="Frames per Task")
priority: int = SettingsField(50, title="Priority")
group: str = SettingsField("", title="Group")
limit_groups: list[LimitGroupsSubmodel] = SettingsField(
default_factory=list,
title="Limit Groups",
)
concurrent_tasks: int = SettingsField(
1, title="Number of concurrent tasks")
department: str = SettingsField("", title="Department")
use_gpu: bool = SettingsField("", title="Use GPU")
job_delay: str = SettingsField(
"", title="Delay job",
placeholder="dd:hh:mm:ss"
)
use_published: bool = SettingsField(True, title="Use Published scene")
asset_dependencies: bool = SettingsField(True, title="Use Asset dependencies")
workfile_dependency: bool = SettingsField(True, title="Workfile Dependency")
multiprocess: bool = SettingsField(False, title="Multiprocess")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be very nice to get description argument set for each of these.

Comment on lines 289 to 291
enabled: bool = SettingsField(True)
optional: bool = SettingsField(title="Optional")
active: bool = SettingsField(title="Active")
Copy link
Contributor

Choose a reason for hiding this comment

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

With all the other attributes removed from these models we can now make one EnableStateModel class instead that we use for all of these making this file much smaller.

Copy link
Member Author

@kalisp kalisp Nov 11, 2024

Choose a reason for hiding this comment

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

The question is if these jobs should be actually optional at all as if they would be disabled, submit publish job will fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

The question is if these jobs should be actually optional at all as if they would be disabled, submit publish job will fail.

I guess the idea would have been that it maybe would allow to disable just the farm publishing support for an instance - but there creators definitely are not configured that way. So I agree - may make no sense to be optional, or even be disabled, currently?

dln_job_info.add_render_job_env_var()

# already collected explicit values for rendered Frames
if not dln_job_info.Frames:
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually think this is a place where we should not be setting this in a Deadline specific way but have it accessible on the instance even outside of the job info. So Frames would be build up from instance.data["frames"] instead so that it could turn out to be non-Deadline specific. However, not critical for now and we can do that later. Might be worth a TODO in code or an issue after this PR.

server/settings/publish_plugins.py Outdated Show resolved Hide resolved
@BigRoy BigRoy added the type: enhancement Improvement of existing functionality or minor addition label Oct 21, 2024
productName might not change, this condition is clear
kalisp and others added 2 commits November 20, 2024 12:17
Co-authored-by: Jakub Trllo <[email protected]>
Co-authored-by: Jakub Trllo <[email protected]>
Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

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

Tested in Unreal and Maya

@LiborBatek
Copy link
Member

LiborBatek commented Nov 21, 2024

@kalisp I dont see any addon settings when built the addon package....it used to work with older commits (when first time tested)

??

image

however renders went through the DL fine and been integrated on AYON...LGTM

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.

Been tested again in blender and maya and both works well...

note: I had experienced DL addon settings were not available to me (package built issue?) so this should be checked if an issue or not.

…support-of-Frame-List' into feature/AY-6789_Render-instance-support-of-Frame-List
Doesnt make much sense on ContextPlugin and it is not working on Pyblish
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.

I think these are no longer needed in fusion submitter settings.

server/settings/publish_plugins.py Outdated Show resolved Hide resolved
server/settings/publish_plugins.py Outdated Show resolved Hide resolved
@LiborBatek
Copy link
Member

@kalisp now addon settings been visible...so all good on this front!

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.

@iLLiCiTiT, Should we add a minor bump to his PR?

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.

For information, This PR is now documented in ynput/ayon-documentation#301

Should be bumped to release when available
…support-of-Frame-List' into feature/AY-6789_Render-instance-support-of-Frame-List
@kalisp kalisp merged commit 59b8fa3 into develop Nov 22, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Render product publish: use same Deadline options as other DCCs ChunkSize in Houdini is ignored
7 participants