getavalon / core

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

More flexible templates #371

Closed mkolar closed 5 years ago

mkolar commented 5 years ago

PR relates to both #308 and #351

New Functionality

This PR allows Avalon to work with arbitrary hierarchies in the system. This is currently used in 2 places.

Filling work path when launching an app and in 'workfiles' app and getting the path of published representation during loading.

Implementation

Getting representation path

Instead of hardcoding what keys we allow to be used in the template and also which template has to be used for publishing (as it is currently), store all the information we need directly on the representation. Proposed solution uses three possibilities to get the path which we try in sequence until one is successful.

  1. Get representation['data']['path'] directly. We assume this is pre-filled full path to the file. If the folder exists on the disk, we use this
    1. get representation['data']['template'] and format it with representation['context']. During this process we make sure that root from the context get's replaced with registered root in avalon to keep it dynamic.
    2. fallback to current way if none of the above was successful.

example of what representation can look like in the db then

id: 5c41dcd19e4ee0671c7e594a
name: "exr"
parent : 5c41dcd19e4ee0671c7e5949
type : "representation"
dependencies : Array
context : Object
    subset: "render_writeDefault"
    family: "write"
    hierarchy: "sequences\sq01"
    version: v003
    project: 
        code: "mln"
        name: "super_project"
    task: "compositing"
    asset: "sh010"
    representation: "exr"
    root: "C:\projects\_AVALON"
    padding: #####
data: Object
    path: "C:\projects\_AVALON\milan_testing\sequences\sq01\sh010\publish\render\compositing\render_writeDefault\v003\mln_sh010_compositing_v003_render_writeDefault.#####.exr"
    template : "{root}/{project[name]}/{hierarchy}/{asset}/publish/{family}/{task}/{subset}/{version}/{project[code]}_{asset}_{task}_{version}_{family}_{subset}.{padding}.{representation}

Formatting work path

The issue here was allowing any hierarchy to be filled into the path without putting too many restriction at what keys (sequence, shot, episode etc) can be used. The solution is introducing concept of AVALON_HIERARCHY which get's constructed on the fly from new parents data on the given asset.

asset['data']['parents'] is expected to be a correctly ordered list of all the hierarchical parents of the asset. This list get's parsed and joined with os.path.sep ending up as a simple string that can be used in the template as {hierarchy} key.

We've used this in production for a while now, but it needs to be tested for backwards compatibility of course. As far as we can tell, nothing should break on previous projects, but better safe than sorry.

We found this feature crucial as different studios can have some seriously different preferences on what their folder structure looks like :). The implementation itself of course is up for discussion. The priority of individual ways of getting the path from representation for example. I find storing the absolute path in data useful as it might later allow extra checks to happen.

BigRoy commented 5 years ago

Just a note, not a direct comment on your code of implementation.

More dynamic paths I think would be warmly welcomed when it doesn't complicate things too much - since it allows for more customization for a specific studio. We just need to ensure that the template we give and allow isn't something that would easily break and introduces issues for simple pipelines or newcomers. (E.g. just storing the full path in a multiple OS pipeline will tend to introduce issues.)

mkolar commented 5 years ago

(E.g. just storing the full path in a multiple OS pipeline will tend to introduce issues.)

very true. it's bothering me too. we should probably make the stored full path as that last resort if all else fails.

mottosso commented 5 years ago

Hey guys, should we merge this?

mkolar commented 5 years ago

Now I believe we can. I've changed the order of resolution based on the comments and it is now.

  1. Get template from representation['data']['template'] and data from representation['context']. Then format template with the data.
  2. Get template from project['config'] and format it with default data set
  3. Get representation['data']['path'] and use it directly

If any is successful, the following won't be tried. 1 is the new way of getting all needed from representation, 2. is exactly the same as before and 3. is fallback to a full path potentially stored on the representation.

I've also added this into the docstring. We're quite happy with this and have it deployed in multiple places now.

tws0002 commented 5 years ago

Hi Mkolar,

i having this error

Running action: maya2018
Traceback (most recent call last):
  File "K:\test\avalon-setup\git\launcher\launcher\control.py", line 418, in trigger_action
    popen = action.process(api.Session.copy())
  File "K:\test\avalon-setup\git\core\avalon\pipeline.py", line 405, in process
    environment = self.environ(session)
  File "K:\test\avalon-setup\git\core\avalon\pipeline.py", line 316, in environ
    workdir = _format_work_template(template, session)
  File "K\test\avalon-setup\git\core\avalon\pipeline.py", line 935, in _format_work_template
    "hierarchy": session['AVALON_HIERARCHY'],
KeyError: 'AVALON_HIERARCHY'
avalon.py: Finishing up..
iLLiCiTiT commented 5 years ago

Hi Mkolar,

i having this error

Running action: maya2018
Traceback (most recent call last):
  File "K:\test\avalon-setup\git\launcher\launcher\control.py", line 418, in trigger_action
    popen = action.process(api.Session.copy())
  File "K:\test\avalon-setup\git\core\avalon\pipeline.py", line 405, in process
    environment = self.environ(session)
  File "K:\test\avalon-setup\git\core\avalon\pipeline.py", line 316, in environ
    workdir = _format_work_template(template, session)
  File "K\test\avalon-setup\git\core\avalon\pipeline.py", line 935, in _format_work_template
    "hierarchy": session['AVALON_HIERARCHY'],
KeyError: 'AVALON_HIERARCHY'
avalon.py: Finishing up..

@mkolar should we also use:

"hierarchy": session.get('AVALON_HIERARCHY', ''),

in _format_work_template for case when hierarchy is empty?

mkolar commented 5 years ago

@mkolar should we also use: "hierarchy": session.get('AVALON_HIERARCHY', ''), in _format_work_template for case when hierarchy is empty?

yes this should be fixed. we didn't catch it before

mottosso commented 5 years ago

Mentioning #417 here, as this is one of those features that are important but difficult to test. For a test to pass in each permutation of this functionality, we need:

  1. No project template, no representation template and no representation path, returning None, and what to do with that; this is up to tools currently to visualise to the user.
  2. No project template, no representation template, but a representation path, and how that should work cross-platform.
  3. No project template, but representation template. Pretty straight forward
  4. Finally, a project template, the prior default and current fallback.

In each case, we need both a database entry per pair, but also existing paths on the file-system, as these won't return anything unless the path actually exists; unsure of whether that's actually the responsibility of this function, but am not sure how else to "automatically" cycle through the options (or whether this is actually better suited for handling by tools that can visualise this).

mottosso commented 5 years ago

I made a mistake on this one, the paths returned currently are the directories, not filenames! :O

Original

https://github.com/pypeclub/avalon-core/blob/da55d0f4f601e613a03e05ee3db33eaece19ec0f/avalon/pipeline.py#L1214

Refactored https://github.com/getavalon/core/blob/9b89842047fec06d221f178191004544d5660837/avalon/pipeline.py#L1227-L1229

Fixing this now.