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

Deadline: Simplify GlobalJobPreLoad logic #2605

Merged
merged 9 commits into from
Feb 18, 2022
Merged

Deadline: Simplify GlobalJobPreLoad logic #2605

merged 9 commits into from
Feb 18, 2022

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Jan 28, 2022

Brief description

This simplifies the logic the Deadline plug-in requires to retrieve the OpenPype Executable.

  • No need to re-save/update the Job to inject OpenPype Executable - the setting is now loaded live from the OpenPype plug-in.
  • The OpenPype Event Plug-in was only used for injecting the OpenPype Executable info into the job - since that's redundant now, that has been removed. That also means you now only have to set the executable in the regular Deadline OpenPype plug-in (not the Event plug-in).
  • I've removed the 'invalidate cache' code of getting the Job since now the Job doesn't need to be altered/re-saved we don't need to really invalidate any caches anyway.

Would love to hear whether this would move it in a good direction or whether this is not preferred?

The Event Plug-in also had some "ftrack" related functions which was basically doing nothing currently. I'm not sure if there might be customers out there how do have that logic altered instead and thus the Event Plug-in might still have a place. Something to consider at least.

@BigRoy BigRoy mentioned this pull request Jan 31, 2022
Merged
@kalisp
Copy link
Member

kalisp commented Feb 11, 2022

I am all up for removing the event, I really hated it anyway.

@BigRoy BigRoy marked this pull request as ready for review February 12, 2022 09:13
@kalisp kalisp self-requested a review February 14, 2022 13:29
kalisp
kalisp suggested changes Feb 14, 2022
View reviewed changes
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.

Could you also please purge all mentions of DL event in vendor/readme.md and website/docs/module_deadline.md?

@BigRoy
Copy link
Collaborator Author

BigRoy commented Feb 14, 2022

Could you also please purge all mentions of DL event in vendor/readme.md and website/docs/module_deadline.md?

Sure can! Like this?

@BigRoy BigRoy requested a review from kalisp February 15, 2022 10:17
@BigRoy
Copy link
Collaborator Author

BigRoy commented Feb 17, 2022

Any chance we can get the Build/Docs deployment to run again @kalisp @mkolar ?

It seems doc building is failing on GitHub:

Run cd website
yarn install v1.22.17
[1/4] Resolving packages...
[2/4] Fetching packages...
error @docusaurus/core@2.0.0-beta.15: The engine "node" is incompatible with this module. Expected version ">=14". Got "12.22.10"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Seems we will need to update node to 14.x?
Here: > https://github.com/pypeclub/OpenPype/blob/3bc8ae4e3b34cd980db9aad8b47b9437cfd78e3b/.github/workflows/documentation.yml#L23
And on line 44.
I believe that'd be updating Node.js

Not sure if that'd break anything else however.

(Copied from Discord)

kalisp
kalisp approved these changes Feb 17, 2022
View reviewed changes
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.

Looks great.

I will keep it unmerged for now to not hide issue with documentation build. After that I would merge it.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Feb 18, 2022 •
edited
Loading

With #2760 merged should these doc builds now pass @mkolar ?

Or does it only take from the local branch and should we merge latest develop into this branch?

Because it appears to still fail.

@mkolar
Copy link
Member

mkolar commented Feb 18, 2022

It should indeed work now, but you need to merge develop to this branch. Github actions are picked from currently checked out branch.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Feb 18, 2022

It should indeed work now, but you need to merge develop to this branch. Github actions are picked from currently checked out branch.

@mkolar Now the check suddenly is gone and thus no documentation build is checked?

@mkolar mkolar added the type: enhancement Enhancements to existing functionality label Feb 18, 2022
@kalisp kalisp merged commit 93c2422 into ynput:develop Feb 18, 2022
@kalisp
Copy link
Member

kalisp commented Feb 18, 2022

Lets merge this one and observe next PRs.

@BigRoy BigRoy mentioned this pull request Mar 2, 2022
Merged
@BigRoy BigRoy deleted the deadlineissues branch March 20, 2024 15:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kalisp kalisp kalisp approved these changes

Assignees

@BigRoy BigRoy

Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BigRoy @kalisp @mkolar