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

Review: Integrate reviewables to AYON #790

Merged
merged 19 commits into from
Jul 30, 2024

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Jul 19, 2024

Changelog Description

Integrate plugin does integrate reviewables to AYON server.

Additional info

Upload reviewables to AYON server during integration. The method is not yet available in ayon-python-api so it had to be implemented there. Also modified default ExtractReview arguments to more suite needs of server, but previous defaults should work too.

I did have issues with ExtractBurnins, that is probably because of my dev environment, please make sure the testing happens with burnins.

There is one possible issue that should be resolved in future, which is that if there is only review representation without representation to integrate, it is actually not uploaded to server because the version is not integrated (as would not have any representations). This topic needs more thinking and usually happens only in edge cases...

Testing notes:

  1. Make sure you have AYON server 1.3.0 or higher.
  2. Create package and upload it to server.
  3. Make sure you have added tag "webreview" in ExtractReview settings.
  4. Start host of your choice.
  5. Run publishing with some review.
  6. The review should be uploaded to AYON server.

You can also change the settings of ExtractReview to new/old defaults and start from step 3 again (based on what did you test first).

@iLLiCiTiT iLLiCiTiT marked this pull request as ready for review July 19, 2024 15:40
@ynbot ynbot added size/S type: feature Adding something new and exciting to the product labels Jul 19, 2024
Copy link
Collaborator

@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.

Requires updated AYON API!

The integrator plug-in didn't work for me initially:

Filepath:
E:\dev\ayon-core\client\ayon_core\plugins\publish\integrate.py

Traceback:
Traceback (most recent call last):
File "E:\dev\ayon-core\client\ayon_core\pipeline\publish\lib.py", line 257, in publish_plugins_discover
module = import_filepath(abspath, mod_name)
File "E:\dev\ayon-core\client\ayon_core\lib\python_module_tools.py", line 38, in import_filepath
module_loader.exec_module(module)
File "", line 883, in exec_module
File "", line 241, in _call_with_frames_removed
File "E:\dev\ayon-core\client\ayon_core\plugins\publish\integrate.py", line 8, in 
from ayon_api import (
ImportError: cannot import name 'RequestTypes' from 'ayon_api' (C:\Program Files\Ynput\AYON 1.0.2\dependencies\ayon_api\__init__.py)

Which was fixed by picking a new launcher version 1.0.3 and the dependency package - but I didn't get any compatibility issues with 1.0.2 and the older dependency package when setting the bundle?

Might be nice if in frontend we can streamline this so the error is caught earlier?

Especially because with this plug-in failing to load the publish APPEARS to succeed (no errors) but there's just no integration... so no actual publish into AYON database at all.

Side note: If you set up the dev mode and are running against the 1.0.2 console using e.g. "C:\Program Files\Ynput\AYON 1.0.2\ayon_console.exe" but with set AYON_USE_DEV=1 I saw NO error or warning whatsoever that it was actually still running 1.0.2 even though my full bundle was set to 1.0.3. It was still using 1.0.2 dependencies package, etc.


Once that was fixed:

Works ✅

I was able to publish a reviewable from DCCs:

  • review from Maya,
  • review from Blender didn't work but it was because for whatever reason extract playblast crashes my blender
  • review from Houdini
  • local render from Fusion.
  • plate publish from Tray Publisher

image

I didn't test any Deadline render setups though because I don't have a good test setup for that currently.

Should the logic move into a Publish plug-in of its own?

Due to the reviewable upload the integration process does become a bit slower - which makes sense.
I just wonder whether it makes sense to move the logic into a plug-in of its own.

  • It can be easily taken out/swapped out for other logic.
  • It's clear it runs in isolation then.
  • It could be easily offloaded to e.g. a farm job?
  • The publisher UI would show the plug-in name for what it's doing, e.g. show "Uploading Reviewables" bottom left.
  • If the plug-in were then made optional, the user could easily enable/disable the push to AYON for certain reviewables?

Reviewables being uploaded 🥳
image

@BigRoy
Copy link
Collaborator

BigRoy commented Jul 20, 2024

Question with regards to Hero versions

Currently hero versions can also be played as reviewable on the web frontend.
But publishing currently, does not add a reviewable to a hero version. Should it?
Also, if it were to add it - what happens if we publish again a new version. Should it replace the old reviewable for it?
Does it even make technically sense to have HERO version be reviewable? Or?

Tagging @mkolar @Innders

Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

I can confirm sucessful Blender 4.2.0 LTS review upload to Ayon.
image

I have ftrack enabled at the same time though and the review was not uploaded to ftrack, nor version created. Only status has been changed
image

@krishnaavril
Copy link

krishnaavril commented Jul 22, 2024

Just testing this code with Nuke, It's working! Can see the video review in Ayon in dev mode,

I tried to add this code in production, then the publishes are not happening at all, there is no version folder being created in publish folder.
ClipWindowsGIF

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Jul 22, 2024

Requires updated AYON API!

Not anymore.

I saw NO error or warning whatsoever that it was actually still running 1.0.2 even though my full bundle was set to 1.0.3. It was still using 1.0.2 dependencies package, etc.

AYON python api is in AYON launcher, not in dependency package.

Should the logic move into a Publish plug-in of its own?
Due to the reviewable upload the integration process does become a bit slower - which makes sense.
I just wonder whether it makes sense to move the logic into a plug-in of its own.

It can be easily taken out/swapped out for other logic.
It's clear it runs in isolation then.
It could be easily offloaded to e.g. a farm job?
The publisher UI would show the plug-in name for what it's doing, e.g. show "Uploading Reviewables" bottom left.
If the plug-in were then made optional, the user could easily enable/disable the push to AYON for certain reviewables?

Not saying we can't move it to separate addon, but not because of mentioned reasons.

  • Reviewables are usually marked for delete, so they are not integrated as representations and are deleted at the end of publish process. So you can't really off-load it for certain workflows. You would still need some overall logic how to depart certain plugins to different process, which we don't have now. (Bigger topic. In brainstorm stage.)
  • You can turn off the logic by not adding webreview tag.
  • Can't tell about having this feature as optional? Should artist tell if review should be uploaded?

@BigRoy
Copy link
Collaborator

BigRoy commented Jul 22, 2024

AYON python api is in AYON launcher, not in dependency package.

well, the error was still there - so even though my AYON dev was set to "launcher 1.0.3" I got no error anywhere that it was actually running 1.0.2 in dev mode because I happened to launch through 1.0.2 console... Might need an obvious error log there?

Reviewables are usually marked for delete, so they are not integrated as representations and are deleted at the end of publish process. So you can't really off-load it for certain workflows. You would still need some overall logic how to depart certain plugins to different process, which we don't have now. (Bigger topic. In brainstorm stage.)

👍

In a way I suppose the same goes for uploads to Shotgrid/Ftrack (no idea whether those run after integration or before?) bit I guess that means each of those should also be able to 'control' whether it has processed the reviewable and should still need integrating or not?

Anyway - all are good topics and questions - maybe not critical for this 0.1 version of this feature, but definitely worth spawning off separate issues if we're merging this.

I tried to add this code in production, then the publishes are not happening at all, there is no version folder being created in publish folder.

Could this be because the reviewables are indeed "deleted" @iLLiCiTiT and hence @krishna8008 was also expecting them integrated but they were now considered reviewables and thus they are not integrated anymore.

@krishna8008 what files were you expecting in that published version? The EXRs? or the converted review mov?
Could you share reproducable steps of how you've configured your Nuke setup and what you're publishing?

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Jul 22, 2024

In a way I suppose the same goes for uploads to Shotgrid/Ftrack (no idea whether those run after integration or before?) bit I guess that means each of those should also be able to 'control' whether it has processed the reviewable and should still need integrating or not?

After integrate, so they can use integration path, but they must run before cleanup plugins too.

Could this be because the reviewables are indeed "deleted" @iLLiCiTiT and hence @krishna8008 was also expecting them integrated but they were now considered reviewables and thus they are not integrated anymore.

Could be, but not related to this PR. It just added option to upload reviewable to AYON.

@fabiaserra
Copy link
Contributor

fabiaserra commented Jul 22, 2024

if there is only review representation without representation to integrate, it is actually not uploaded to server because the version is not integrated (as would not have any representations).

Why are "review" representations not actual representations? Does this mean that reviewables won't be available to be loaded in the different DCCs as a representation?

@iLLiCiTiT
Copy link
Member Author

Why are "review" representations not actual representations? Does this mean that reviewables won't be available to be loaded in the different DCCs as a representation?

They're not all the time, but you can add (I think it is) "delete" tag to it to prevend integration. It is handy if you want to upload reviewables to AYON/ftrack/shotgrid and not really integrating them as representation.

@fabiaserra
Copy link
Contributor

They're not all the time, but you can add (I think it is) "delete" tag to it to prevend integration. It is handy if you want to upload reviewables to AYON/ftrack/shotgrid and not really integrating them as representation.

In our case what we upload to Flow (and what we would similarly upload to AYON potentially) we always want them as representations as users validate the review from Nuke constantly as well, not just from the production tracking tool

@iLLiCiTiT
Copy link
Member Author

In our case what we upload to Flow (and what we would similarly upload to AYON potentially) we always want them as representations as users validate the review from Nuke constantly as well, not just from the production tracking tool

Ok, that is valid, but we also do allow to not integrate them.

@fabiaserra
Copy link
Contributor

Ok, that is valid, but we also do allow to not integrate them.

Yeah that's fair, but I wanted to make sure it's an optional thing and so your statement that if there is only review representation without representation to integrate, it is actually not uploaded to server because the version is not integrated (as would not have any representations) it's not always the case? In our case the review representation should always be integrated right?

@dee-ynput
Copy link

I have ftrack enabled at the same time though and the review was not uploaded to ftrack, nor version created. Only status has been changed

Please be sure to test with active production trackers (at least ftrack).
Unless this comment from @m-u-r-p-h-y is fixed, we cannot release.

@m-u-r-p-h-y
Copy link
Member

I have ftrack enabled at the same time though and the review was not uploaded to ftrack, nor version created. Only status has been changed

Please be sure to test with active production trackers (at least ftrack). Unless this comment from @m-u-r-p-h-y is fixed, we cannot release.

it seems the issue is missing defaults for Blender collect family in Ftrack. Will re-test again

Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

uploading the review from blender works as expected.

we may need to investigate the ftrack in another issue/PR

@iLLiCiTiT iLLiCiTiT merged commit 354e9af into develop Jul 30, 2024
3 checks passed
@iLLiCiTiT iLLiCiTiT deleted the feature/integrate-reviewables-to-ayon branch July 30, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S type: feature Adding something new and exciting to the product
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants