More flexible templates #371

mkolar · 2019-02-03T21:29:47Z

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.

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

houndci-bot · 2019-02-03T21:29:54Z

avalon/pipeline.py

+            if os.path.isdir(pathdir):
+                output = path
+
+        # format representation template with context if 'path' wasn't a success


line too long (80 > 79 characters)

BigRoy · 2019-02-04T09:07:51Z

avalon/pipeline.py

@@ -967,6 +956,9 @@ def update_current_task(task=None, asset=None, app=None):
    _session = Session.copy()
    _session.update(changed)
    changed["AVALON_WORKDIR"] = _format_work_template(template, _session)
+    changed['AVALON_HIERARCHY'] = os.path.sep.join(
+            asset_document['data']['parents']


This will break pipelines that don't have the parents data, right? This should allow for backwards compatibility.

true. we'll put a .get() there with default of empty string to make sure it always resolve to something.

BigRoy · 2019-02-04T09:12:02Z

avalon/pipeline.py

@@ -1003,6 +995,7 @@ def _format_work_template(template, session=None):
        "root": registered_root(),
        "project": session["AVALON_PROJECT"],
        "silo": session["AVALON_SILO"],
+        "hierarchy": session['AVALON_HIERARCHY'],


Will this error out on pipelines who haven't set AVALON_HIERARCHY yet?

Should this be in the Session schema if it's required?

What is this value and when would you set it? How would you compute it for any asset?

yes I'd say that making is required in the schema is reasonable. however there is a caveat here. Because it hold multiple key in the path joined together with a path.sep, it will be quite hard to parse the opposite way. So for example what you are using in your colorbleed config (get context from path) will likely not work.

AVALON_HIERARCHY resolve to a list of parents joined by path.sep so for example If asset is in this hierarchy: project1/assets/characters/hero, then the template would be `{project/{hierarchy}/{asset}.

hierarchy = 'assets/characters`

This needs to be made optional, as per the comments.

BigRoy · 2019-02-04T09:13:48Z

avalon/pipeline.py

-        "app": Session.get("AVALON_APP", ""),
-        "task": Session.get("AVALON_TASK", "")
-    })
+    output = None


Could you add in the docstring the order of which it resolves the paths in the simplest sense. That way one can easily figure it out without reading the lines of code. It seems to have 3 ways of getting the data and I think it would be good to keep that order and reasoning clear for anyone trying out the function without looking at the code implementation.,

This seems to have been addressed.

BigRoy · 2019-02-04T09:18:54Z

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 · 2019-02-11T23:23:18Z

(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 · 2019-03-27T09:00:45Z

Hey guys, should we merge this?

…ath.

houndci-bot · 2019-03-28T08:17:11Z

avalon/pipeline.py

@@ -1190,6 +1186,13 @@ def switch(container, representation):
 def get_representation_path(representation):
    """Get filename from representation document

+    There are three ways of getting the path from representation which are tried


line too long (80 > 79 characters)

mkolar · 2019-03-28T08:21:44Z

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

Get template from representation['data']['template'] and data from
representation['context']. Then format template with the data.
Get template from project['config'] and format it with default data set
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 · 2019-04-01T08:49:23Z

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 · 2019-04-01T18:23:42Z

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 · 2019-04-03T13:07:01Z

@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

This seems solid, I'll take care of the remaining few comments now.

mottosso · 2019-08-20T16:28:14Z

avalon/pipeline.py

@@ -1003,6 +995,7 @@ def _format_work_template(template, session=None):
        "root": registered_root(),
        "project": session["AVALON_PROJECT"],
        "silo": session["AVALON_SILO"],
+        "hierarchy": session['AVALON_HIERARCHY'],


This needs to be made optional, as per the comments.

mottosso · 2019-08-20T16:28:56Z

avalon/pipeline.py

-        "app": Session.get("AVALON_APP", ""),
-        "task": Session.get("AVALON_TASK", "")
-    })
+    output = None


This seems to have been addressed.

mottosso · 2019-08-20T16:30:46Z

avalon/pipeline.py

-        "task": Session.get("AVALON_TASK", "")
-    })
+    output = None
+    try:


This try/except block is too broad. At the moment, if the first formatting fails, the second doesn't happen. Also that first if output is None: is superflous, can we remove that?

…bitary_hierarchy_templates # Conflicts: # avalon/pipeline.py

mottosso · 2019-08-20T17:58:13Z

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:

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.
No project template, no representation template, but a representation path, and how that should work cross-platform.
No project template, but representation template. Pretty straight forward
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 · 2019-08-21T18:26:36Z

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

core/avalon/pipeline.py

Lines 1227 to 1229 in 9b89842

    
           dirname = os.path.dirname(path) 
        
           if os.path.isdir(dirname): 
        
               return os.path.normpath(dirname)

Fixing this now.

iLLiCiTiT and others added 3 commits February 1, 2019 14:39

path and path template is loaded from representation first

e2ffaa4

added hierarchy to session

33e0853

add comments

13cf365

houndci-bot reviewed Feb 3, 2019

View reviewed changes

fix comment length

c62a447

BigRoy reviewed Feb 4, 2019

View reviewed changes

add fallback in case parents data is missing

22187d3

Add docstring and change the order of resolution for representation p…

1e63c15

…ath.

houndci-bot reviewed Mar 28, 2019

View reviewed changes

fix long line in docstring

da55d0f

mottosso reviewed Aug 20, 2019

View reviewed changes

mottosso added 2 commits August 20, 2019 18:49

Merge remote-tracking branch 'origin/master' into cleanup/PYPE-180_ar…

1827d43

…bitary_hierarchy_templates # Conflicts: # avalon/pipeline.py

Increment minor version

2650c43

mottosso merged commit 9340d88 into getavalon:master Aug 20, 2019

mottosso mentioned this pull request Aug 21, 2019

Resilience against missing paths #422

Open

mottosso added a commit that referenced this pull request Aug 21, 2019

Fix regression from #371

1d89899

davidlatwe mentioned this pull request Dec 13, 2019

Resilience against missing paths #424

Closed

mkolar deleted the cleanup/PYPE-180_arbitary_hierarchy_templates branch December 19, 2019 17:31

jasperges mentioned this pull request Feb 6, 2020

Optionally initialize Session['AVALON_HIERARCHY'] #525

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More flexible templates #371

More flexible templates #371

mkolar commented Feb 3, 2019 •

edited by davidlatwe

Loading

houndci-bot Feb 3, 2019

BigRoy Feb 4, 2019

mkolar Feb 11, 2019

BigRoy Feb 4, 2019

mkolar Feb 11, 2019

mottosso Aug 20, 2019

BigRoy Feb 4, 2019

mottosso Aug 20, 2019

BigRoy commented Feb 4, 2019

mkolar commented Feb 11, 2019

mottosso commented Mar 27, 2019

houndci-bot Mar 28, 2019

mkolar commented Mar 28, 2019

tws0002 commented Apr 1, 2019

iLLiCiTiT commented Apr 1, 2019

mkolar commented Apr 3, 2019

mottosso left a comment

mottosso Aug 20, 2019

mottosso Aug 20, 2019

mottosso Aug 20, 2019

mottosso commented Aug 20, 2019

mottosso commented Aug 21, 2019

More flexible templates #371

More flexible templates #371

Conversation

mkolar commented Feb 3, 2019 • edited by davidlatwe Loading

New Functionality

Implementation

Getting representation path

Formatting work path

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BigRoy commented Feb 4, 2019

mkolar commented Feb 11, 2019

mottosso commented Mar 27, 2019

Choose a reason for hiding this comment

mkolar commented Mar 28, 2019

tws0002 commented Apr 1, 2019

iLLiCiTiT commented Apr 1, 2019

mkolar commented Apr 3, 2019

mottosso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mottosso commented Aug 20, 2019

mottosso commented Aug 21, 2019

mkolar commented Feb 3, 2019 •

edited by davidlatwe

Loading