feat: Run pre-GUI hooks in the VRED render submitter - #148
feat: Run pre-GUI hooks in the VRED render submitter#148leon-li-inspire wants to merge 4 commits into
Conversation
Call deadline-cloud's run_pre_gui_hooks before building SubmitJobToDeadlineDialog so studios can pre-populate dialog fields. VRED has no on-disk job bundle at pre-GUI time, so hooks are sourced from DEADLINE_HOOKS_DIR only (bundle_dir=None); the confirmation prompt is skipped when settings.auto_accept is set. The merged hook output is applied with deadline-cloud's generic apply_pre_gui_output. RenderSubmitterUISettings has no .parameters list (unlike the standalone submitter's JobBundleSettings), so that helper writes name/description onto the settings and routes every hook parameter into the dialog's shared parameter values. The hooks run in _create_submitter_dialog, so they fire each time the submitter opens. Bumps the deadline floor to >= 0.60.1 (the release that ships run_pre_gui_hooks / PreGuiHookContext / apply_pre_gui_output via aws-deadline/deadline-cloud#1255; they are absent from 0.60.0) and adds headless unit tests that exercise the real core helper against VRED's settings dataclass. Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com>
e605d85 to
e7f7513
Compare
…ocstring Address review feedback on the pre-GUI hook tests: - Add TestCreateSubmitterDialogPreGuiWiring, unit coverage for the DCC-owned control flow in _create_submitter_dialog that the helper-only tests missed: the auto_accept branch selecting confirm_callback (qt_hook_confirmation vs None), the PreGuiHookContext construction (bundle_dir=None, submitter_name, job_name, parameters), and that apply_pre_gui_output is applied onto the same render_settings / shared values the dialog is seeded with. A regression that dropped the apply call, flipped the auto_accept condition, or mispopulated the context now fails a unit test. deadline-cloud is mocked, so the tests run headless. - Group the existing helper tests under TestApplyPreGuiOutputForVred. - Fix the module docstring: apply_pre_gui_output first ships in 0.60.1 (the floor this change sets), not 0.60.0 — it is absent from 0.60.0. Aligns with the pyproject floor. Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com>
Mirror the review fixes made on the Maya submitter (deadline-cloud-for-maya#437): - Extract _pre_gui_hook_confirm_callback(parent): moves the settings.auto_accept branch out of _create_submitter_dialog into a small module-level helper so it can be unit-tested headlessly. - Drop the multi-line dependency comment above the deadline pin in pyproject.toml (the >= 0.60.1 floor speaks for itself now that the release is out). - Tests: add TestPreGuiHookConfirmCallback covering both auto_accept branches, including one that exercises the real qt_hook_confirmation and asserts the QMessageBox confirmation actually fires (parented to the window, Yes -> proceed) rather than only checking a non-None callback was selected. Slim the dialog- wiring test to the context build + confirm-callback delegation + apply wiring. Not applicable to VRED (unlike Maya): VRED imports pre_gui_hooks at module top with no `# pylint: disable=import-error`, and its tests never used pytest.importorskip, so the "stale pylint disable" and "drop the skip" review items have no VRED equivalent. Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com>
| # no on-disk job bundle at this point, so hooks are sourced from DEADLINE_HOOKS_DIR only | ||
| # (bundle_dir=None), gated by settings.allow_environment_hooks. The confirmation prompt is | ||
| # skipped when auto_accept is set; otherwise the standard dialog is shown. | ||
| pre_gui_output = run_pre_gui_hooks( |
There was a problem hiding this comment.
From the 3dsmax version, it seems we need to add a try-catch?
There was a problem hiding this comment.
From the Github scan bot on the 3dsmax PR: aws-deadline/deadline-cloud-for-3ds-max#262
When a user declines the hook confirmation prompt, run_pre_gui_hooks does not return an empty result — it raises DeadlineOperationCanceled (see deadline-cloud pre_gui_hooks.run_pre_gui_hooks: if confirm_callback is not None and not confirm_callback(sources): raise _DeadlineOperationCanceled(...)).
This call is at the top level of show_job_bundle_submitter, and neither this function nor its callers (run_ui.show_ui → show_job_bundle_submitter()) wrap it in a try/except. So a user simply clicking No on the confirmation dialog will propagate an unhandled exception up into 3ds Max, surfacing as an error/traceback rather than quietly aborting the submitter open.
Consider catching DeadlineOperationCanceled around the run_pre_gui_hooks call and returning early (no dialog), matching the "user canceled" intent. The same wrap would also contain any exception raised from within a studio hook script during execute_pre_gui_hooks.
What was the problem/requirement? (What/Why)
The VRED submitter had no way for studios to pre-populate the submitter dialog before it opens. The other DCC submitters (Maya, Nuke) are gaining a pre-GUI hook integration so studios can run a hook — sourced from
DEADLINE_HOOKS_DIR— that seeds job name, description, and parameters beforeSubmitJobToDeadlineDialogis built. VRED should have parity.What was the solution? (How)
In
VREDSubmitter._create_submitter_dialog, after the Conda shared-parameter values are assembled and before the dialog is constructed:run_pre_gui_hooks(PreGuiHookContext(...)). VRED has no on-disk job bundle at pre-GUI time, sobundle_dir=None— hooks come fromDEADLINE_HOOKS_DIRonly (gated bysettings.allow_environment_hooks).settings.auto_acceptis set; otherwise the standard Qt confirmation dialog (qt_hook_confirmation) is shown.apply_pre_gui_output.RenderSubmitterUISettingshas no.parameterslist (unlike the standalone submitter'sJobBundleSettings), so the helper writesname/descriptiononto the settings and routes every hook parameter into the dialog's shared parameter values.Sets the
deadlinefloor to>= 0.60.1, < 0.61, the release that shipsrun_pre_gui_hooks/PreGuiHookContext/apply_pre_gui_output.What is the impact of this change?
DEADLINE_HOOKS_DIRunset (or env hooks disabled) there are no hook sources,run_pre_gui_hooksreturns{}, andapply_pre_gui_outputis a no-op. The dialog is seeded with the same Conda packages/channels as before._create_submitter_dialog, so they fire each time the submitter is opened.How was this change tested?
black --check,ruff check, andpy_compileall pass.test/unit/test_pre_gui_hooks.pyexercise the real coreapply_pre_gui_outputagainst VRED's actualRenderSubmitterUISettings(name/description assignment, parameter routing into shared values, empty/partial-output no-ops, and the premise that the settings dataclass has no.parameterslist).test_create_submitter_dialog, which now flows through the realrun_pre_gui_hooks).Please run the integration tests and paste the results below
Not run locally — the VRED integration tests require a running VRED instance. Requesting a CI run.
If
installer/was modified or a file was added/removed fromsrc/, then update the installer tests and post the test results belowNo files were added or removed from
src/(onlyvred_submitter.pywas modified) andinstaller/was not touched, so the installer tests are unaffected.Was this change documented?
Docstrings/comments were added at the hook invocation site explaining the
bundle_dir=Noneenv-only sourcing and whyapply_pre_gui_outputroutes all parameters to shared values for VRED. No README/DEVELOPMENT/schema changes are needed.Is this a breaking change?
No API-level breaking change. Dependency note: this raises the
deadlinefloor to>= 0.60.1. The requiredrun_pre_gui_hooks/PreGuiHookContext/apply_pre_gui_outputsymbols landed via aws-deadline/deadline-cloud#1255 (now merged) and first ship in deadline-cloud 0.60.1 — they are absent from 0.60.0, which is why the floor is>= 0.60.1rather than>= 0.60.0. That release is available, so this PR is no longer blocked on the dependency. The companion Maya and Nuke PRs should use the same>= 0.60.1floor.