getavalon / core

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

Expose "open_latest_workfile" method for public use. #382

Closed tokejepsen closed 5 years ago

tokejepsen commented 5 years ago

Goal

To enable developers to open the latest workfile on launch of application.

Motivation

Instead of hacking a solution together using the methods inside of the workfiles app, thought it better to expose a dedicated method that might benefit others.

BigRoy commented 5 years ago

Honestly, I'm not a fan... it's a bit too high-level without lower level being publicly available too. In my repo I have a setup where the work files api is exposed per host and I'd recommend first going that way, where it does:

#psuedocode
import avalon.api

host = api.registered_host ()
# understand where the work files are, dependent on software plus pipeline?
path = avalon.api.get_workfiles()[-1]

host.open (path)

Or something along those lines? From a dev perapective it just makes more sense to expose the files from the workfiles easily and how to open those in the host aside of only exposing to open the latest work file?

So, yes. Great idea. But the way it's exposed is too specific for avalon core.

tokejepsen commented 5 years ago

Or something along those lines?

Sure yeah, that sounds great. Just been catering for my own needs atm, which it to open the latest workfile on application launch, so any improvements to that is good.

More specifically I get why the host should live in the Avalon API, but should the get_workfiles not live in the workfiles module?

mottosso commented 5 years ago

More specifically I get why the host should live in the Avalon API, but should the get_workfiles not live in the workfiles module?

Ah, didn't spot you were exposing it via workfiles. I think we should keep any external access to Avalon accessible via api rather than via the package __init__. Anything in api is expected to be well maintained and last forever (so take care!), and anything not in api can't be relied upon for external use.

What Avalon does internally however is nobodies business, but it seems like you expect external users and developers building tools on-top of Avalon to use this, in which case api is the place to be.

tokejepsen commented 5 years ago

Anything in api is expected to be well maintained and last forever (so take care!), and anything not in api can't be relied upon for external use.

Good point. Will update this PR with the changes.

tokejepsen commented 5 years ago

Actually on that point, should the tools be exposed in the API as well?

tokejepsen commented 5 years ago

@BigRoy I agree with this method, but it seems like we should get https://github.com/getavalon/core/issues/365 sorted first then?

BigRoy commented 5 years ago

Once #387 is merged likely it's best to close off this PR. However, @tokejepsen please still have a critical eye at whether the availability of "getting the last workfile" is simple enough or we still need to explicitly expose something to the core api.

mottosso commented 5 years ago

With #387 merged, did we still think this should not get merged?

I agree with Roy that it's a little too high-level, even magical. Maybe this is better suited as a sorting function to the workfiles model/view itself? Such that you can click "Open Latest", as in, whichever is last in the list. Then sorting is both visible to the user and customisable, either by custom function implemented in a config, or by the user clicking on a relevant section of the view, like ascending modified date.

tokejepsen commented 5 years ago

As stated the goal was for developers to launch the latest workfile for users instead of presenting the workfiles app on launch. One button click less and no waiting for the workfiles app. The workfiles app already preselects the last modified file in the list.

I think all the talk about API exposure needs to be settled before this one can continue.

mottosso commented 5 years ago

One button click less and no waiting for the workfiles app.

Can you provide a usecase and/or example of how you envision the function to be used? I thought it was meant for either this GUI, or someone elses GUI, but I'm not seeing how that would reduce the number of clicks.

tokejepsen commented 5 years ago

Can you provide a usecase and/or example of how you envision the function to be used? I thought it was meant for either this GUI, or someone elses GUI, but I'm not seeing how that would reduce the number of clicks.

The idea was to open the latest workfile on launch on the application, rather than launching the application then open the latest workfile. Launching either using Avalon Launcher or Ftrack.

mottosso commented 5 years ago

The idea was to open the latest workfile on launch on the application, rather than launching the application then open the latest workfile.

Ahaaa. But isn't that a little invasive? What if you wanted to open Maya without automatically loading the latest file? What if the file is huge and takes several minutes?

The idea is sound, how about we combine this with #412 and have the Context Manager show up on Maya launch instead?

tokejepsen commented 5 years ago

Ahaaa. But isn't that a little invasive? What if you wanted to open Maya without automatically loading the latest file? What if the file is huge and takes several minutes?

This is subjective, so would be implemented per config, and hopefully the config will also make it project based.

mottosso commented 5 years ago

Ok, I see what you mean. You want this method exposed for external configurations to Avalon. I can see how that makes sense, and how other common convenience functions have a place in Avalon too.

Because there's no limit to how much convenience we can bake into Avalon, I'd like to make a strict separation between what is convenience and what is core functionality. Convenience being built on-top of core. David's suggestion to add a avalon.util seems a good place for this, something we can add to freely and something that can grow, and worst case require a manual implementation using core features if convenience breaks. Something not bound by the "everlasting support" of the public API.

Let's move this PR into a new util.py module, would you like to take the lead on that @tokejepsen?

tokejepsen commented 5 years ago

Let's move this PR into a new util.py module, would you like to take the lead on that @tokejepsen?

My time schedule means I wont be able to look at this for weeks, but yeah.