Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow path dict for publish #816

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gplsteph
Copy link

SG TK does its best to retrieve a LocalStorage from a path for publishes, however there are many use cases where multiple storages could have the same root path on a platform, but different root paths for other platforms.

A typical case is just having a drive letter on Windows, but different mount points for the other platforms, for example:

project1:
    windows_path: P:\
    mac_path: /Volume/project1

project2
    windows_path: P:\
    mac_path: /Volume/project2

These changes allow to explicitly pass a { "local_storage", "relative_path"} as the path parameter to register_publish for cases where these values are known in advance.
Unit tests were added to cover the added feature.

Added the ability to pass a dictionary as a publish path with `local_storage` ad `relative_path` values.
Added unit tests for explicit local storage publishing.
Copy link
Contributor

@barbara-darkshot barbara-darkshot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gplsteph !

I like this idea but I'd request some code changes.

Instead of using a dictionary to pass the local_storage to the register_publish() method, I'd introduce a new kwargs that I could then use in the _calc_path_cache() method called by _create_published_file() to be able to filter by root name

This will avoid to have duplicate piece of code and also, with some slight modifications, have the same behavior for the find_publish() method

https://github.com/shotgunsoftware/tk-core/blob/master/python/tank/util/shotgun/publish_creation.py#L250
image

https://github.com/shotgunsoftware/tk-core/blob/master/python/tank/util/shotgun/publish_creation.py#L303
image

https://github.com/shotgunsoftware/tk-core/blob/master/python/tank/util/shotgun/publish_creation.py#L519
image

https://github.com/shotgunsoftware/tk-core/blob/master/python/tank/util/shotgun/publish_creation.py#L803
image

https://github.com/shotgunsoftware/tk-core/blob/master/python/tank/util/shotgun/publish_creation.py#L822
image

This is how we could extend the find_publish() method to have the same behavior by specifying the local storage name
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants