Pre and post loader hooks

[kalisp]

Changelog Description

This PR introduces new concept of Pre/Post Loader Hooks which could be used by to enhance or modify loading processes without need of modifying official loaders.

The idea is for custom addons to be able to implement their own PreLoadHookPlugin | PostLoadHookPlugin which would be triggered before/after Loader action.

Additional info

Each addon must register its folder with pre/post hooks like this:

    register_loader_pre_hook_plugin_path(loader_hook_dir)
    register_loader_post_hook_plugin_path(loader_hook_dir)

I am testing it with this dummy custom addon my_custom_addon-1.0.0.zip

Testing notes:

1. start with this step
2. follow this step

[ynbot]

Task linked: OP-6870 Plugin hooks - Loader and Scene Inventory

[BigRoy]

I wonder if new hook plug-ins here make more sense than just an EventSystem 😕 Seems somewhat overkill here.

[kalisp]

I wonder if new hook plug-ins here make more sense than just an EventSystem 😕 Seems somewhat overkill here.

Had the same question, but they should be blocking, I dont think you can achieve that with events.

[BigRoy]

I wonder if new hook plug-ins here make more sense than just an EventSystem 😕 Seems somewhat overkill here.

Had the same question, but they should be blocking, I dont think you can achieve that with events.

The Event system in AYON is blocking by default, right @iLLiCiTiT ?

[iLLiCiTiT]

The Event system in AYON is blocking by default, right @iLLiCiTiT ?

Yes it is. But it matters what should happen during the hooks, and what is the reason/use-case to add the hooks?

Because adding hooks (2 classes) to the current crap load api means that we will have to add more backwards compatibility with more classes to be duplicated when the api changes, which will complicate it.

[iLLiCiTiT]

I wanted to ask very similar questions to @BigRoy 's but I wanted to wait for this first https://github.com/ynput/ayon-core/pull/1222#issuecomment-2782443206 . We really need to know what is the use-case. It's all related to current crap api. Loader plugins are also "actions" (delivery etc.), so I guess the pre/post hook needs to know "some" context, but because current api does not actually tell you anything about "if", "where" or "what" actually happened it's just a random classes with functions that do get triggered in various occations that probably won't be important for the hooks.

[kalisp]

Summoning @antirotor to describe more details about https://github.com/ynput/ayon-core/issues/928

[antirotor]

Summoning @antirotor to describe more details about #928

Clients are changing some behaviour of some loaders. This should help them stay "vanilla" and still add some custom behaviour. For example - you load layout into your scene and you want to immediately lock all transforms so artist cannot accidentally move stuff around. All logic for the loader is the same, you just want to execute few lines of code after the load is complete.

There are also often customization about hierarchy where loaded stuff should end up - if you want to customize this - to add loaded model into very specific structure, again - the whole loader logic is the same, you just need to move loaded items somewhere.

[iLLiCiTiT]

There are also often customization about hierarchy where loaded stuff should end up - if you want to customize this - to add loaded model into very specific structure, again - the whole loader logic is the same, you just need to move loaded items somewhere.

So how will these changes help if you don't know what happened in load?

[iLLiCiTiT]

Also important note is that this PR is adding 2 new classes and at about 10 new functions just to register and discover them, changing api functions for load. All of that will make backwards compatibility even harder when we begin with new load api.

[antirotor]

We should move with this forward. So, what about one class for hooks (PrePostLoadHook) with pre_load and post_load methods - hook it on loader class name and monkey-patch loader .load() so it executes those hook methods before and after? Make it as simple as possible.

So how will these changes help if you don't know what happened in load?

You can go around by listing the scene in pre_load and post_load and you'll get what loader pulled in and then you can manipulate the result.

[kalisp]

Vastly simplified logic.

Currently monkey patched method is added to Loader classes who are compatible to any registered PrePostLoaderHookPlugin during discover_loader_plugins.

[BigRoy]

@kalisp please re-request my review when you think it's ready for a hopefully final review.

I personally think having remove update and switch access would be very nice.

---

ALSO, have we considered making them context managers? So that we CAN also get to the post hook if e.g. update failed? E.g. there may definitely be cases where you may want to 'wrap' something up that you've done before - even if the core logic failed in-between?

(The question then is if the 'exit' should be ordered in reverse, so that the outer context manager exits last?) ❓ But maybe this is thinking too much of it.

[kalisp]

Implemented update, remove methods.

Solved all other comments.

[kalisp]

Implemented update, remove methods.

Yeah, you are right, it was broken, I misread log and got confused its working on my dummy test. I re tested (and this time double checked myself).

Thanks for the updates, they seem valid :).

[iLLiCiTiT]

Ok, I might be just down beause I had to recently remove all uses of TypedDict from my code because we still support Python 3.7. I believe str | None is working only for python 3.10+ so I guess this will kill any older Maya than 2024 (Maya 2023 has 3.9)? Sorry. I would like to use newer syntax so much...

I didn't notice that. Good point.

[kalisp]

Reordered arguments in pre_, post_ methods.

[iLLiCiTiT]

It is mergable, from my point of view the abstract methods are unnecessary, but I won't be the one who will use it.