getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 48 forks source link

Dedicated Tool Schema #415

Open mottosso opened 5 years ago

mottosso commented 5 years ago

Goal

Facilitate arbitrary data in people's MongoDB instances, without compromising Tool expectations and support.

Motivation

Following #406, there was a need to separate data provided by MongoDB and data expected by tools, but instead a decision was made to align everyone's data to the same conventions. This is bound to ruffle some feathers, and doesn't scale to additional data people might want to use. We certainly should not enforce a naming convention and whatever data we absolutely require should be minimal. And even that data should not limit someone from designing a naming convention of choice.

The problem with conflating the two is that there's no limit to the amount of default or expected data a project or asset or what not may provide. Why stop at frameStart and handleStart?

Implementation

What should have happened is that we should have introduced a conversion layer.

For example.

# Unique to your studio and convention
data_in_your_mongodb = {
  "assets": [
    {"name": "vector", "niceName": "Vector"},
    {"name": "shot1005", "niceName": "Final Battle", "data": {"frameStart": 1000, "frameEnd": 1200}}
  ]
}

# Implemented by you
def asset_to_loader(asset):
  return {
    "start_frame": asset["data"]["frameStart"],
    "label": asset["niceName"],
    ...
  }

In this case, whenever Loader (or any tool) requires data from Mongo, it would pass it through this function. If it exists. If not, the data is passed straight in, as it is right now. We could then have a similar function for each document type in Mongo.

def project_to_loader():
def version_to_loader():

Rather than having a translator per tool, there could potentially be a generic Avalon translator, that returns data for all tools, some optional and some required.

def project_to_avalon(project):
def asset_to_avalon(asset):

This would then also be where we could handle validation of said data, to error on missing fields, and where we could document what kind of data is supported.

Doing this, it doesn't matter what your data looks like; it's decoupled from the implementation details of Loader. This "translation" layer is something you provide from your context to align your data with the data expected/supported by Loader, and any tool.

In that way, your data can be frameStart, Roy's data can be startFrame and my data can be start_frame, and Avalon is happy either way.

tokejepsen commented 5 years ago

I like it! This would also help with supporting "older" projects that might have an old style of naming convention that your studio would like to transition away from.

BigRoy commented 5 years ago

I understand what you are going after, but to me this doesn't sound like the best way.

Yes,

  1. We should keep as little data required as possible. It doesn't always need to be there.
  2. We should allow to let everyone pick their own custom data.

But additionally I think it's amazing if we can find some 'standard' way to do some things so that moving forward there's so much more code that we could share. Otherwise it might get very hard to translate someone's code snippets to one of your own.

Anyway, how about approaching this a different yet somewhat similar way.

Say we allow the Loader (or other tools) to be overridden as to what data they are showing? E.g. we allow a config to override some sort of get_loader_data function that returns an OrderedDict and the view would list it in that order.

This isn't perfect either. Returning data like that is fine for a treeview but e.g. the loader sidebar, what data would that show?

^just some early morning thoughts above.


Anyway, aren't we just shifting the problem from:

To:

Either way you'd need to convert to the naming scheme of the data. It's the avalon database intended for using the avalon workflow. Shouldn't that recommend some sort of avalon standards?


It's early morning so it might be that I need to read this over one more time to really grasp how adding the extra complexity that any one config suddenly needs to write the code to allow the tools to work is the best way to solve this.

mottosso commented 5 years ago

Anyway, aren't we just shifting the problem from:

No, it's about avoiding discussions or standards of naming convention and where data is coming from within the core library. It's perfectly fine to have a config standard; even a standard across multiple config. But it's not for core to decide. Core should be unbiased and facilitate data coming from Mongo, or the filesystem, or the environment, or another DB like Shotgun etc. That conversion function could handle all of that without involving Core.

Maybe we could setup a page for the Colorbleed config, along with documentation and recommended data structures, along with a Colorbleed-translator to go with that?

BigRoy commented 5 years ago

We could, sure. But it would also automatically mean that most of my plug-ins won't be usable by someone else without changing all instance.data things and alike.

What would we do with visualParent? So would one call this function prior to querying the databse so you know what data to query the database for? If we can't query the database then it loses its need to actually be a database. However some might one to "massage" the data for the final result. So you'd then need it to be post-query.

Would love to see a more thorough prototype/example, maybe a PR, but it sounds a bit scary. We are still enforcing a naming convention yet elsewhere. The tools still require a specific convention for which you'll need to massage your data.

I might be too afraid to lose efficiency. I've seen how bad a slow pipeline can be in production and I'm worried that all these "post-conversions and massaging" are just going to add another 0.1s for massaging e.g. 100 publishes in an asset. Anyway, just wanted to state what came up in my head. I'll read back what others think throughout the day.

BigRoy commented 5 years ago

Also, in your original example you have niceName outside of data. As far as I'm aware that should never be recommended to store the config related things outside of data as outside of it that's where it should closely reflect the schemas, inside of data is your custom data.

Or am I misinterpreting how this was intended?

mkolar commented 5 years ago

I'm not particularly keen on this approach either. It feels like adding complexity and extra layer for something that should be super fast, clear and most importantly consistent. I think it's perfectly reasonable to be asking configs to conform to certain naming convention.

Besides core already had requirements for most of this data, because it expected it in different tools. The problem was that even within avalon, they were not consistent.