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

Bugfix: Farm cache publish support newer ayon-deadline versions #164

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

BigRoy
Copy link
Contributor

@BigRoy BigRoy commented Nov 5, 2024

Changelog Description

Set the publish.hou family much earlier to avoid issues with ayon-deadline plug-ins that were ordered earlier than how we set it before this PR in ayon-houdini.

Additional review information

Fixes #138

Testing notes:

  1. Use latest ayon-deadline
  2. Publish farm cache
  3. Should work

@BigRoy BigRoy added the type: bug Something isn't working label Nov 5, 2024
@BigRoy BigRoy self-assigned this Nov 5, 2024
MustafaJafar
MustafaJafar previously approved these changes Nov 12, 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.

It works. Thank you.

@MustafaJafar MustafaJafar self-requested a review November 12, 2024 22:21
@BigRoy
Copy link
Contributor Author

BigRoy commented Nov 13, 2024

it was wrong.

Actually - it was right. @MustafaJafar both create from context and this one being at -0.5 made it ambiguous which would run first. This could be -0.499 sure, but it MUST run after that one because that is what creates the instances from the Create Context.

@MustafaJafar MustafaJafar self-requested a review November 13, 2024 14:02
@MustafaJafar
Copy link
Contributor

it was wrong.

Actually - it was right. @MustafaJafar both create from context and this one being at -0.5 made it ambiguous which would run first. This could be -0.499 sure, but it MUST run after that one because that is what creates the instances from the Create Context.

So, the -0.5 is valid but it conflicts with CollectFromCreateContext.

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.

It works.
But, do we need to make this plugin run at earlier time? the CollectJobInfo has a late value pyblish.api.CollectorOrder + 0.420


I wonder if CollectFarmCacheFamily plugin can be replaced by using the get_publish_families method.

@BigRoy
Copy link
Contributor Author

BigRoy commented Nov 13, 2024

I wonder if CollectFarmCacheFamily plugin can be replaced by using the get_publish_families method.

Sort of. But it shouldn't be there ALL the time, but only if farm is enabled on the instance. So we'd need to make that dynamic (which is possible now with value changed callbacks) however then the Deadline plug-ins wouldn't "dynamically" respond to that by default and the deadline plug-ins would need to actively listen for changes on the families to dynamically update as soon as the instance's families changes through such a toggle in the UI.

But, do we need to make this plugin run at earlier time? the CollectJobInfo has a late value pyblish.api.CollectorOrder + 0.420

Honestly - I'd prefer to have it the EARLIEST so that it's entirely clear that is doesn't really depend on anything else. Whereas there may be more things relying on targeting publish.hou families over time.

But making this a changed callback since ynput/ayon-core#994 is merged could avoid this plug-in as described above.

@BigRoy
Copy link
Contributor Author

BigRoy commented Nov 13, 2024

Merging this - because it fixes a bug. We can turn it into changed callback in separate PR.

@BigRoy BigRoy merged commit 45b7806 into ynput:develop Nov 13, 2024
1 check passed
@MustafaJafar
Copy link
Contributor

But, do we need to make this plugin run at earlier time? the CollectJobInfo has a late value pyblish.api.CollectorOrder + 0.420

I forgot about that context plugin CollectDeadlineJobEnvVars that has pyblish.api.CollectorOrder order. and this PR fixed the issue where the submitted jobs didn't have the proper env variables to run.

so it does fix a bug thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collect cache farm instances and Deadline plugins order
2 participants