From 81835aec6b9ee9ae5c94bdccb0e48580d467f406 Mon Sep 17 00:00:00 2001 From: Stephane Deverly Date: Mon, 26 Jul 2021 11:23:24 +0200 Subject: [PATCH 1/3] Ability to explicitly specify a local_storage for publishing. 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. --- python/tank/util/shotgun/publish_creation.py | 91 +++++++++++++++---- tests/util_tests/test_publish.py | 95 ++++++++++++++++++++ 2 files changed, 168 insertions(+), 18 deletions(-) diff --git a/python/tank/util/shotgun/publish_creation.py b/python/tank/util/shotgun/publish_creation.py index 42ac40ead5..ea6a272af0 100644 --- a/python/tank/util/shotgun/publish_creation.py +++ b/python/tank/util/shotgun/publish_creation.py @@ -459,11 +459,83 @@ def _create_published_file( # set the associated project data["project"] = context.project + if isinstance(path, dict): + # If a dictionary, we expect the "local_storage" and "relative_path" keys + # to be populated. We just do some basic sanity checks to make sure the + # publish will not fail straight away. + + # Check if the shotgun server supports the storage and relative_path parameters. + supports_specific_storage_syntax = ( + hasattr(tk.shotgun, "server_caps") + and tk.shotgun.server_caps.version + and tk.shotgun.server_caps.version >= (7, 0, 1) + ) + if not supports_specific_storage_syntax: + raise TankError( + "Your SG server version does not support storage and relative_path parameters " + "for publishes." + ) + # Check needed values are populated + if not path.get("local_storage") or not path.get("relative_path"): + raise TankError( + "Both 'local_storage' and 'relative_path' values must be set in publish path dictionary." + ) + # Normalize input path to remove double slashes etc. + norm_path = ShotgunPath.normalize(path["relative_path"]) + if os.path.isabs(norm_path): + raise TankError( + "Path %s is an absolute path, not a relative path to a local storage." % norm_path + ) + + # Convert the abstract fields to their defaults + norm_path = _translate_abstract_fields(tk, norm_path) + # Name of publish is the filename + data["code"] = os.path.basename(norm_path) + # Normalize to only use forward slashes. + norm_path = six.ensure_str(norm_path.replace("\\", "/")) + data["path"] = { + "relative_path": norm_path, + "local_storage": path["local_storage"], + } + data["path_cache"] = norm_path + + else: + # Assume a string, extract SG data from it. + data.update(_get_publish_data_from_path(tk, path)) + + # now call out to hook just before publishing + data = tk.execute_core_hook( + constants.TANK_PUBLISH_HOOK_NAME, shotgun_data=data, context=context + ) + + if dry_run: + # add the publish type to be as consistent as possible + data["type"] = published_file_entity_type + log.debug( + "Dry run. Simply returning the data that would be sent to SG: %s" + % pprint.pformat(data) + ) + return data + else: + log.debug("Registering publish in ShotGrid: %s" % pprint.pformat(data)) + return tk.shotgun.create(published_file_entity_type, data) + +def _get_publish_data_from_path(tk, path): + """ + Parse the given path and extract path related SG data for publishing. + + :param tk: :class:`~sgtk.Sgtk` instance. + :param str path: The path to the file or sequence we want to publish. If the + path is a sequence path it will be abstracted so that + any sequence keys are replaced with their default values. + :returns: A dictionary with path related SG data set. + """ # Check if path is a url or a straight file path. Path # is assumed to be a url if it has a scheme: # # scheme://netloc/path # + data = {} path_is_url = False res = urllib.parse.urlparse(path) if res.scheme: @@ -612,24 +684,7 @@ def _create_published_file( "url": file_url, "name": data["code"], # same as publish name } - - # now call out to hook just before publishing - data = tk.execute_core_hook( - constants.TANK_PUBLISH_HOOK_NAME, shotgun_data=data, context=context - ) - - if dry_run: - # add the publish type to be as consistent as possible - data["type"] = published_file_entity_type - log.debug( - "Dry run. Simply returning the data that would be sent to SG: %s" - % pprint.pformat(data) - ) - return data - else: - log.debug("Registering publish in ShotGrid: %s" % pprint.pformat(data)) - return tk.shotgun.create(published_file_entity_type, data) - + return data def _translate_abstract_fields(tk, path): """ diff --git a/tests/util_tests/test_publish.py b/tests/util_tests/test_publish.py index 520804432f..3a902c424b 100644 --- a/tests/util_tests/test_publish.py +++ b/tests/util_tests/test_publish.py @@ -302,6 +302,101 @@ def test_freeform_publish(self, create_mock): self.assertEqual(sg_dict["path"], path_dict) self.assertTrue("pathcache" not in sg_dict) + @patch("tank_vendor.shotgun_api3.lib.mockgun.Shotgun.create") + def test_explicit_local_storage_publish(self, create_mock): + """ + Tests handling publishes where the path is 'local_storage' and 'relative_path' + dictionary. + """ + values = [ + { + "local_storage": self.storage_2, + "relative_path": "path/to/file.txt" + }, + { + "local_storage": self.storage_3, + "relative_path": "path/to/file.txt" + }, + + ] + # We should get a SG version error + with self.assertRaisesRegex( + tank.util.ShotgunPublishError, + "Your SG server version does not support storage and relative_path parameters", + ): + publish_data = tank.util.register_publish( + self.tk, self.context, values[0], self.name, self.version, dry_run=True + ) + + # Mock SG server capabilities + class server_capsMock: + def __init__(self, version): + self.version = version + + # TODO: should we unset this? + self.mockgun.server_caps = server_capsMock(version=(7, 0, 1)) + + # Check missing values + with self.assertRaisesRegex( + tank.util.ShotgunPublishError, + "Both 'local_storage' and 'relative_path' values must be set", + ): + publish_data = tank.util.register_publish( + self.tk, + self.context, + {"local_storage": self.storage_2}, + self.name, + self.version, + dry_run=True + ) + with self.assertRaisesRegex( + tank.util.ShotgunPublishError, + "Both 'local_storage' and 'relative_path' values must be set", + ): + publish_data = tank.util.register_publish( + self.tk, + self.context, + {"relative_path": "path/to/file.txt"}, + self.name, + self.version, + dry_run=True + ) + # Check invalid path + with self.assertRaisesRegex( + tank.util.ShotgunPublishError, + "is an absolute path, not a relative path to a local storage", + ): + publish_data = tank.util.register_publish( + self.tk, + self.context, + { + "relative_path": os.path.join( + os.path.join(self.project_root, "path/to/file.txt") + ), + "local_storage": self.storage_2, + }, + self.name, + self.version, + dry_run=True + ) + + # Test valid values and results + for local_path in values: + publish_data = tank.util.register_publish( + self.tk, self.context, local_path, self.name, self.version, dry_run=True + ) + self.assertIsInstance(publish_data, dict) + + tank.util.register_publish( + self.tk, self.context, local_path, self.name, self.version + ) + + create_data = create_mock.call_args + args, kwargs = create_data + sg_dict = args[1] + self.assertEqual(sg_dict["path"], local_path) + self.assertEqual(sg_dict["path_cache"], local_path["relative_path"]) + def test_publish_errors(self): """Tests exceptions raised on publish errors.""" From 3ceab114e384a626138d16c9f9a3625b81ed0646 Mon Sep 17 00:00:00 2001 From: Stephane Deverly Date: Mon, 26 Jul 2021 11:32:20 +0200 Subject: [PATCH 2/3] Docstrings --- python/tank/util/shotgun/publish_creation.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/python/tank/util/shotgun/publish_creation.py b/python/tank/util/shotgun/publish_creation.py index ea6a272af0..8c210f5603 100644 --- a/python/tank/util/shotgun/publish_creation.py +++ b/python/tank/util/shotgun/publish_creation.py @@ -52,11 +52,14 @@ def register_publish(tk, context, path, name, version_number, **kwargs): number. This is used to group together publishes in Shotgun and various integrations. - If the path matches any local storage roots defined by the toolkit project, - it will be uploaded as a local file link to Shotgun. If not matching roots - are found, the method will retrieve the list of local storages from Shotgun - and try to locate a suitable storage. Failing that, it will fall back on a - register the path as a ``file://`` url. For more information on + The path can either be a string or, for advanced use cases, a dictionary with + `local_storage` and `relative_path` values explicitly set. + + If the path is a string and matches any local storage roots defined by the + toolkit project, it will be uploaded as a local file link to Shotgun. If not + matching roots are found, the method will retrieve the list of local storages + from Shotgun and try to locate a suitable storage. Failing that, it will fall + back on a register the path as a ``file://`` url. For more information on this resolution logic, see our `Admin Guide `_. @@ -161,6 +164,8 @@ def register_publish(tk, context, path, name, version_number, **kwargs): :param path: The path to the file or sequence we want to publish. If the path is a sequence path it will be abstracted so that any sequence keys are replaced with their default values. + The path can be a string or a dictionary with `local_storage` and + `relative_path` values. :param name: A name, without version number, which helps distinguish this publish from other publishes. This is typically used for grouping inside of Shotgun so that all the @@ -377,6 +382,8 @@ def _create_published_file( :param path: The path to the file or sequence we want to publish. If the path is a sequence path it will be abstracted so that any sequence keys are replaced with their default values. + The path can be a string or a dictionary with `local_storage` and + `relative_path` values. :param name: A name, without version number, which helps distinguish this publish from other publishes. This is typically used for grouping inside of Shotgun so that all the From ea2ebbf2e2ce01b005e026936e588266d2846936 Mon Sep 17 00:00:00 2001 From: Stephane Deverly Date: Mon, 26 Jul 2021 12:22:06 +0200 Subject: [PATCH 3/3] Flake8 --- python/tank/util/shotgun/publish_creation.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/tank/util/shotgun/publish_creation.py b/python/tank/util/shotgun/publish_creation.py index 8c210f5603..a6df17fdac 100644 --- a/python/tank/util/shotgun/publish_creation.py +++ b/python/tank/util/shotgun/publish_creation.py @@ -527,6 +527,7 @@ def _create_published_file( log.debug("Registering publish in ShotGrid: %s" % pprint.pformat(data)) return tk.shotgun.create(published_file_entity_type, data) + def _get_publish_data_from_path(tk, path): """ Parse the given path and extract path related SG data for publishing. @@ -693,6 +694,7 @@ def _get_publish_data_from_path(tk, path): } return data + def _translate_abstract_fields(tk, path): """ Translates abstract fields for a path into the default abstract value.