Chore: Load api

[iLLiCiTiT]

Changelog Description

Base of new load api definition.

Additional info

I got stuck, that's why we're here.

This is a draft that is meant to trigger discussion related to load api enhancements. It is NOT in final state, there are missing pieces. The draft is meant to discuss general concept, not to fine-tune code details. Suggest better approach instead of asking why it's so bad.

Introduction

Our current load api is still unchanged since avalon. It is simple, effective but sometimes we do suffer from that simplicity. Loader plugins are class objects, usually hard to be re-used, they don't have context they lack control over how the stuff they produce, work with is handled afterwards and more other small issues connected to it. Loader plugins are also used for UI actions (no difference in load plugin and action).

NOTE

With loader actions #1421 we already started to separate "actions" and "loaders".

Goal

Define new api for load related logic.

• Load.
• Remove.
• Change representation.

Sum of expected features

I need confirmation for few of them. Checked ones are confirmed. Not checked need validation. Striked won't be added.

• Load plugins are context aware -> they have access to host and other load plugins, they can share data between load plugins if necessary. They're not single-use class based object, but can be re-used.
• Load plugins are responsible of handling what is loaded and how it is then collected.
• It must be possible to run everything headlessly.
• Switch load plugin. New load plugin must confirm it does support the switch.
• Load multiple representations at one time -> I think this is mandatory from my point of view, but it might complicate few things here are there.
• Load plugin can have "options" for load/update/remove/switch -> more below.

Ask for more if you have more.

Possible requirements

• Add option to register callbacks to addons. Current load api have hooks, it would be nice to be able to avoid them and allow addons to listen to load context changes. I don't think there is a use for pre hooks (change my mind). It would mean that addons could register listening to load context events, probably using some predetermined method on an addon?

Structures

LoadContext

Wrapper for load api. Takes care about discovery and access to load plugins, supports api methods to collect/change/remove containers and allows to register callbacks to a specific events.

During collection phase allows to store "shared data" so load plugins can share data from scene. It is common that the load metadata are stored the same way in 99% cases of DCC integration.

ContainerItem

Holds metadata about container for load api and for UI purposes. There are expected data we want to keep stored with container that are "generic" as they are needed for core pipeline purposes, but there has to be an option to store data that are related to the DCC that should be stored with the scene metadata, and there also might be data that are important for working with the container, but are not stored to the scene metadata.

Required information

Immutable - Can't be changed after container creation.

• container id - Unique identifier of the container, can be uuid or path to node or something else.
• project name - From which project is representation.
• representation id - Representation id.
  Mutable - Can be changed during container lifetime.
• load plugin identifier - Which load plugin created/collected container. (Not sure if switch should remove old container and create new one?)
• version locked - Is version locked -> Affects validation of outdated containers.
• parent container id - Optional parent container it for UI purposes.
• label - How is container visualized in UI.
• namespace - Group name of container in UI (I think).

Change of any of the values will trigger an event that value changed. Some of them don't have to be stored to the scene.

Transient data
Load plugin can store any objects here, that should be maintained only by the plugin itself. For example node object from the scene. Undefined structure and won't trigger any event.

Scene data
Additional data that are stored to the scene. Could be layer/node id/name for DCCs where container metadata are not stored on the "item" that represents loaded representation but to generic scene metadata. Expected to be json serializable (type-wise).

Additional scene data are a little bit problematic. The main question is if we should handle changes of values and trigger an event when it happens. If we should track changes then it must be custom dictionary object without type hints, but we'd be able to handle value changes. If we won't track the changes then we can let host inherit from ContainerItem and let it add typed object (or own dictionary), in that case transient and scene data can be ditched out completelly.

My guts say that we don't have to capture the changes, but we should provide default option for additional scene data and we should also provide transient data to give opinion how we think it should work. But I might be wrong.

Few thoughs

The issue with making loading more advanced is that some of the usages would suffer. For example current workfile template builder can load representations easily, but if a load plugin requires loader action because it is too advanced it might not be able to use the load plugin in template builder -> we might need to look into template builder implementation and add more features there to support feature parity with old load mechanism.

I was thinking that the load plugins won't be automatically shown in loader UI as they did before. Instead they would require to have a loader action (there can be one pre-implemented action in core that would make them work as they did before). That would allow to easily accomplish simple load scenarions but also support advanced load mechanism, which might require to call multiple load plugins at once (e.g. load of model, texture and animation in one step etc.). Also related to attribute definitions.

Current questions

Load methods and arguments

In current state of the draft load plugins have load_representations, change_representations and remove_containers (load, update and remove). Load and change are representation related, for representation compatibility check is is_representation_compatible -> singular so it can return bool.

All of them don't have additional arguments. But they should have, so if they're e.g. called from an action, they might require more "additional" context. But how to define the arguments to be able to call just "load representation" in automation without any additional knowledge about the load plugin. Right now the idea is that there would be optional key word argument additional_data.

Another question is: Do loaders need to define attribute definitions like they did before? E.g. load the tree 10 000 times. From my point of view this is based on the fact if we'll use loader actions to show the load plugins in UI, because in that case the loader action could ask that and would pass the value to the load using 'additional_data`. That would also partially solve possible issues with template builder.

[antirotor]

One thing to consider regarding the containers:

Some hosts like Unreal are facing situations where loaded container can be:

• just part of the project but not used (instantiated as an actor) in any level
• can be instantiated in one or more levels along with other containers of the same product, but different versions

So you can load model_foo version 1, 4, and 10. You have levels inside Unreal project called sh010, sh020 and sh030. In level sh010, model v4 is used 3 times. In the level sh020 is used once as v4 but twice as v10. And in the level sh030 just v10 is used. So v1 is never used but loaded in project.

Artist must be able to handle cases where he needs to replace all loaded versions with another one, only versions used in certain levels, only selection.

It gets even more complicated with layouts and versioning assets inside. When you load layout, you should be able to version its content independently - but when you do, I think the version of the loaded layout should be... well... invalidated - or in some state that would flag that the loaded version of the layout is not actually that version because it's content was modified.

---

All of it is pointing maybe to some need of addressing hierachies of loaded containers within the API - so when loader Foo changes container within another container, something will happen - either the Load Context would handle these cases from above, or loaders could define callbacks to be called from below?

Maybe worth of some brainstorming.

[antirotor]

One more point that is related to the open questions - wouldn't it be simpler to base the loading api around representation traits only and forget the legacy system? That means more context about representations for the loaders - how the loaders can decide if and what to do with specific representation? @philippe-ynput

[iLLiCiTiT]

One more point that is related to the open questions - wouldn't it be simpler to base the loading api around representation traits only and forget the legacy system? That means more context about representations for the loaders - how the loaders can decide if and what to do with specific representation?

Isn't that what override of is_compatible_representation would do? You'd still need to implement that per loader anyway, doing that using a method also allows backwards compatibility (which will be needed for some time). I'm honestly against the filtering sysmtem we had in old loader plugins, you couldn't easily define and apply settings and change it dynamically so it does support some backwards compatibility if needed. From my experience we will need it.

All of it is pointing maybe to some need of addressing hierachies of loaded containers within the API - so when loader Foo changes container within another container, something will happen - either the Load Context would handle these cases from above, or loaders could define callbacks to be called from below?

Maybe worth of some brainstorming.

I don't think we have to implement this now. But what we should be prepared for is how the hierarchy would look like -> and based on that maybe how it would be defined.

Should it be like nested like Container B has a parent Container A? Would that be shown in a different view?

Related to loaded assets vs instanced asset -> Would it make sense that current container would be 2 different items? I don't want to bend it to Unreal, but it might make sense to have an "asset" and then "instances" of the asset.
Or just treat loaded asset as an instance, and if is e.g. changed to different version/representation, then treat it as change the asset and all instances of the asset too? No change for other hosts, but unreal would return one more container item for each asset, and then would resolve what should really happen based on containers passed into the change_representation method.

[BigRoy]

Some quick notes from the call currently:

• Need a dedicated method to imprint/change data. How do I e.g. update version_freeze data for the LoadPlugin to imprint?
• We need some way to "parent" multiple Containers to others - so perhaps container item may need to be able to return a parent id.
• We may need an is_dirty state for Container Items the Load Plugin can explicitly return for. So that e.g. any 'inner' children of a loaded layout (if altered separately from changing the layout container itself) can be detected and shown in the UI.
• The data stored to the scene: "objectName", "namespace", "name". They are used across DCCs, but they rarely are an actual namespace, etc. So it's up for discussion which of these are required values. Doesn't always have to be stored in the scene (and if it does, maybe that's specific to that loader only?)

Sorry for the mess

[antirotor]

• We may need an is_dirty state for Container Items the Load Plugin can explicitly return for. So that e.g. any 'inner' children of a loaded layout (if altered separately from changing the layout container itself) can be detected and shown in the UI.

Related to this - to extend possibilities of future Scene Inventory tool, it would be cool to add some way to call some callbacks in loaders. Example:

You are listing all loaded containers in Unreal. You would like to see, what containers are "just" loaded and what are actually instantiated in levels. That is something you cannot store on the container, it is runtime information that needs to be collected at that moment.

Depends on design pattern - could be for example Observer/Listener/... (but that would mean adding some logic to ContainerItem)

loader creates ContainerItem on load and registrer its callback method on it:

container = ContainerItem(...)
container.register_update_callback(self.update_container_status) 

Now whenever container is accessed, it will execute registered callback. That callback then check current state and update some information on it, emit event, do something.

This would at the end enable Scene Inventory to show if loaded assets are instantiated (and where) in Unreal. I suppose any DCC can come with similar examples.

This would also allow automations around loaded stuff.

[BigRoy]

@antirotor that example you provide sounds more like something that the LoadPlugin could just store as data? And then perhaps have a columns attribute or some metadata which hints at that it should be shown as column in the UI if it can - so that the frontend can show custom columns?

[antirotor]

@antirotor that example you provide sounds more like something that the LoadPlugin could just store as data?

Not this one - that value must be computed at the time you show it (or you want to use somehow).

When you load something, loader will create container and imprint. But to show if the loaded asset was instatiated (used somewhere) is something that must be determined at exact time. The loader knows what it is loading so those callbacks would be mostly defined by it.

Imagine the similar thing in Houdini, where loader creates some nodes, but until the nodes are actually connected in some way to the rest of the scene, you don't know if it is used. And because the node can have different outputs or it can be linked be expressions or any other logic, it is up to the loader logic to find out.

The usage status is just an example but having this callback/observer/listener logic can be useful for other things - like automating logic on changes of the loaded data.

It could be even solution for invalidating layout instance if it's content has changed. That logic can be implement directly by the layout loader that will be observing it's content and will be notified by the change. The callback would then either calculate and compare checksums or just directly flag itself as "invalid".

[BigRoy]

When you load something, loader will create container and imprint. But to show if the loaded asset was instatiated (used somewhere) is something that must be determined at exact time. The loader knows what it is loading so those callbacks would be mostly defined by it.

Sure - but isn't that exactly what LoadPlugin.collect_containers does. It returns the state, currently.

The usage status is just an example but having this callback/observer/listener logic can be useful for other things - like automating logic on changes of the loaded data.

Convince me - if someone needed particular data about a container, why couldn't it just collect it itself? In what cases are we providing additional data out of the box that will be useful in the majority of cases or useful to show in the UI. And if that's the case - couldn't that just be added in the returned data on LoadPlugin.collect_containers.

Or are you just saying you want to add a layer so that another plug-in can extend that data for any LoadPlugin. So that a studio plug-in can e.g. register their own additional custom data to be shown int he Scene Inventory. I do like that. But I feel Observer/Listener is overkill there? We'd essentially just allow get_containers returns from the API to run any post-process data collection plug-ins on the containers, and we're there?

I feel like I'm stupid again. Can you provide some pseudocode of what you're envisioning?

[antirotor]

You are right about the collect_containers(). Yet, this is method implemented on the Loader itself and if you want to customize it, you would need to override it (and perhaps not just the method but the whole plugin if you want to return something from it, etc.). It is not returning anything currently so it would need to collect AND change data on containers and that smells (not just because it doesn't match the name of the method :))

So lets investigate first the way using collect_containers(). Scenario in pseudo-python code where you would like to know if the container is instantiated somewhere (perhaps like in Scene Inventory):

"""Loader plugin."""

class CoolLoader(LoadPlugin):
    # implementing all necessary abstract methods like `is_representation_compatible()`, etc.
    ...
    
    def collect_containers(self) -> list[ContainerItem]:  # btw this is currenly defined to return None
        _containers = json.loads(my_app_api.get("ayon_containers"))
        collected: list[ContainerItem] = []
        for container in _containers:
            cnt = ContainerItem.from_dict(container)
            cnt.transient_data["active"] =my_app_api.is_active(container["id"])
            collected.append(cnt)
        return collected

and then in consuming tools:

"""Tool to show loaded containers and their "status"."""

# this will execute `collect_containers()` on Loader plugin
load_context = LoadContext()
for container in load_context._containers:  # there is not public access for now?
    print(f'container {container["container_id"]} is {container["transient_data"]["active"]}'"

That's all fine but - this use of transient_data - to pass data between different tools is easy to use but also highly undefined. My point is that we'll get into same issue as with pyblish instance data where you can store anything anywhere but then make sense of it once it gets too complex.
Another point is that to extend this, you need to add more and more logic to the loader responsible for the container. Any change in "consumer" would have to be done also in the "producer" - the Loader. For example - adding new column in scene inventory would need you to add this logic to all loaders?

But that proposed observer/listener/... logic would make this much more flexible - first example doesn't differ that much because all the logic is related to the plugin anyway:

"""Loader plugin."""

class CoolLoader(LoadPlugin):
    # implementing all necessary abstract methods like `is_representation_compatible()`, etc.
    ...
    
     def collect_containers(self) -> list[ContainerItem]:  # btw this is currenly defined to return None
        _containers = json.loads(my_app_api.get("ayon_containers"))
        collected: list[ContainerItem] = []
        for container in _containers:
            cnt = ContainerItem.from_dict(container)
            cnt.register_observer(self.get_active)
            collected.append(cnt)
        return collected

    def get_active(container: ContainerItem) -> None:
        container.transient_data["active"] =my_app_api.is_active(container["id"])

and then in consuming tool:

"""Tool to show loaded containers and their "status"."""

# this will execute `collect_containers()` on Loader plugin
load_context = LoadContext()
for container in load_context.get_containers():  # this would execute the observers
    print(f'container {container["container_id"]} is {container["transient_data"]["active"]}'"

so in there not much difference. But you could run register_observer() from anywhere even another loaders. Also this is simple example but we could have different types of observers (or listener system) for different actions - for tracking changes. ContainerItem could notify observers when it is destroyed (deleted) or some property changes. For example with layouts:

class LayoutLoader(LoadPlugin):

    def load_representations(self, representation_context) -> list[ContainerItem]:
        layout_data = json.load(representation_context.get_representations()[0].get_trait(FileLocation).path)
        # create base container
        layout_container =  ContainerItem(...)
        for asset in layout_data:
            loader_id = self.get_loader_id_for_representation(Representation(asset["representation"]))
            loader = self.load_context.get_plugin(loader_id)
            loaded_container = loader.load_representations(...)  # now I am not sure where to get RepresentationContext here
            loaded_container.register_observer(ContainerItem.Lifecycle.CHANGED, self.invalidate)
    ...

Now I am skipping through it but you hopefully get the point. BTW ContainerItem.Lifecycle.CHANGED could be enum defining exact points in ContainerItem lifecycle where the observers could be notified.

Main advantage is IMO more obvious definition of data passed between and flexibility in adding/modifying.

---

As a side note:

When writing this I've noticed:

• I don't know how RepresentationContext works - .load_representations() expects one RepresentationContext but can load multiple representations. Also (and I know I am constantly bitching about this) I really don't know what to pass to all those *_entity attributes under RepresentationContext. I just really really hate dict[str, Any]
• LoadContext lacks public methods for accessing containers
• Accessing data like container["transient_data"]["whatever"] feels a little bit verbose. It is personal preference but having getter method would allow us to do something about those passed dicts later on.

[antirotor]

And regarding the question opened in the description:

I don't think we need to handle container id and those things. Every loader is responsible for imprinting/collecting containers and all it needs is to return ContainerItem - that's fine.

With the additional arguments to load_representations() -> it would be good to have some example why this should be needed. IMO if we are using all those Context objects (load and representation), no additional arguments should be needed. Definitely not broad like additional_data: dict[str, Any] or just kwargs. What needs to be passed there, from where - and why?

Name and labels - that is important thing. There are two ways , both with some merit. With representation traits, you can define name/label during the publishing. But that is describing the data, not how the loader interprets them. So that should be up to the loader (and loader can decide based on representation itself).

[iLLiCiTiT]

NOTE Transient data are not and won't be tracked at all, transient data are meant for internal load plugin data. For example to store node object from a scene. Not something that a generic logic can handle in terms of capturing changes. (That's why they's called transient and are not typed.)

[iLLiCiTiT]

it would be good to have some example why this should be needed. IMO if we are using all those Context objects (load and representation), no additional arguments should be needed. Definitely not broad like additional_data: dict[str, Any] or just kwargs. What needs to be passed there, from where - and why?

To support what current LoaderPlugin supports with options. That is used e.g. in maya ornatrix load plugin, etc.

I don't think we need to handle container id and those things. Every loader is responsible for imprinting/collecting containers and all it needs is to return ContainerItem - that's fine.

If you're talking about "id": "ayon.load.container" then I agree, if about container id then I think that is mandatory.

[antirotor]

NOTE Transient data are not and won't be tracked at all, transient data are meant for internal load plugin data. For example to store node object from a scene. Not something that a generic logic can handle in terms of capturing changes. (That's why they's called transient and are not typed.

I'd say we shouldn't use direct dict access - we should use public method to get the data - because then, if we want to change where and what is saved, we just change the logic in the method.

[iLLiCiTiT]

I'd say we shouldn't use direct dict access - we should use public method to get the data - because then, if we want to change where and what is saved, we just change the logic in the method.

Please be more specific, it is meant for load plugin internal data, how could we know what the load plugin wants to store there? And we should not care... The load plugin is only one who should access the transient data.

[antirotor]

To support what current LoaderPlugin supports with options. That is used e.g. in maya ornatrix load plugin, etc.

I think this shouldn't be handled by method argument. Loader should expose what options he can work with and get the data through that.

Something like:

class MyLoaderOptions(LoaderOptions):
    instance_number: int
    color: str


class MyLoader(LoadPlugin):
    ...
    @staticmethod
    def get_options_class() -> Type[LoaderOptions]:
        return type(MyLoaderOptions)
        
    def set_options(options: MyLoaderOptions) -> None:
        self._options = options
        

And the code responsible for showing UI of LoaderOptions would then just do something like:

options = loader,get_options_class()
options(**form_data)
loader.set_options(options)

Advantages are that you'll have models for dialogs, you'll get type annotations, etc. Those examples could be written differently, also the options could be optional argument in loader constructor for sure. Just that they should be clearly defined.

If you're talking about "id": "ayon.load.container" then I agree, if about container id then I think that is mandatory.

yes, I agree

[antirotor]

Please be more specific, it is meant for load plugin internal data, how could we know what the load plugin wants to store there? And we should not care... The load plugin is only one who should access the transient data.

my point is that it should be exposed only by getter method and not as public dict property on ContainerItem

I have bigger issue with data where we are "forcing" some structure:

data.update({
            "container_id": container_id,
            "project_name": project_name,
            "representation_id": representation_id,
            "load_plugin_identifier": load_plugin.identifier,
            "version_locked": version_locked,
        })

Meaning that data will have always this. There must be better way?

[iLLiCiTiT]

I have bigger issue with data where we are "forcing" some structure:

That's data not transient_data. And yes, I have already local changes changing that.

I think this shouldn't be handled by method argument. Loader should expose what options he can work with and get the data through that.

I would like to avoid storing data related to single load to the plugin object itself -> can lead to race condition issues, or changing the options to different values from "defaults" and then calling it again expecting it does not behave as before.

# This effectivelly change how any future load works
load_plugin.set_options(...)
load_plugin.load_representation(...)
...somewhere in future e.g. in different function (considering template builder for example)...
# This will still use the options from above, even though it should use defaults
load_plugin.load_representation(...)

[antirotor]

I would like to avoid storing data related to single load to the plugin object itself -> can lead to race condition issues, or changing the options to different values from "defaults" and then calling it again expecting it does not behave as before.

Should new instance of the loader be created for every loading "session"? Btw that is a good topic to discuss. I can imagine one session, where you execute a bunch of loading actions (as items of work, not as LoadActions) and they should then use the same loader instance - for example layout loader will run even hundreds of calls to different loaders that should share the same options. But loading one product and then loading another one (from artist perspective) should trigger completely fresh load.

Related - loader options presets, etc. This gets complicated - again for example in unreal - where loading something involves unreal framework that itself consumes some options. We are already juggling with options predetermined using Settings, options that could/should be changed by artist just before load and some presets (basically the latter but grouped together in enum in UI sense).

[iLLiCiTiT]

Should new instance of the loader be created for every loading "session"?

No. In case you want to load 2 different models in one session you should not need to re-load and re-initialize plugins and re-collect containers again and again, unless you explicitly request it using reset on load context.

But loading one product and then loading another one (from artist perspective) should trigger completely fresh load.

It should, and that's why it should be an argument and not state of the load plugin.

options that could/should be changed by artist just before load and some presets (basically the latter but grouped together in enum in UI sense).

You mean allow user to "override" settings defaults to his default? Possible, with the actions, another reason why they might be useful? But in theory the local override action (to set it up) can be just different action where user would set that up, and the load plugin could use it.

[BigRoy]

Should this include checking whether it can switch to a specific representation? Or is that supposed to be checked prior to this call?

[iLLiCiTiT]

It was meant if it can switch a container, before you choose representation. Representation is then validated with is_representation_compatible.

[BigRoy]

The 'change' terminology sounds very new to the load API that existed. I wonder if this should be update_representation instead? :/

[iLLiCiTiT]

You can change to different (lower) version or switch container of other loader.

EDITED:
I see that I did add switch_containers but it would make more sense to actually use this method instead as you might also want to change the representation at the same time.

[BigRoy]

What if only one of the containers can't switch? Should it stop immediately?

[iLLiCiTiT]

Good question, and I have no idea. Maybe the conversation is not relevant as I remember I wanted to use change_representation instead for this.

[BigRoy]

Any examples?

[iLLiCiTiT]

For example something like layer_name. The idea is/was that scene_data are tracked for changes. Maybe not relevant?