Merge pull request #3583 from pypeclub/feature/integrate_small_issues

General: New Integrator small fixes

openpype/plugins/publish/integrate.py

@@ -9,12 +9,12 @@
 from pymongo import DeleteMany, ReplaceOne, InsertOne, UpdateOne
 import pyblish.api
 
-import openpype.api
 from openpype.client import (
  get_representations,
  get_subset_by_name,
  get_version_by_name,
 )
+from openpype.lib import source_hash
 from openpype.lib.profiles_filtering import filter_profiles
 from openpype.lib.file_transaction import FileTransaction
 from openpype.pipeline import legacy_io
@@ -78,12 +78,6 @@ def get_frame_padded(frame, padding):
  return "{frame:0{padding}d}".format(padding=padding, frame=frame)
 
 
-def get_first_frame_padded(collection):
- """Return first frame as padded number from `clique.Collection`"""
- start_frame = next(iter(collection.indexes))
- return get_frame_padded(start_frame, padding=collection.padding)
-
-
 class IntegrateAsset(pyblish.api.InstancePlugin):
  """Register publish in the database and transfer files to destinations.
 
@@ -168,7 +162,7 @@ class IntegrateAsset(pyblish.api.InstancePlugin):
  # the database even if not used by the destination template
  db_representation_context_keys = [
  "project", "asset", "task", "subset", "version", "representation",
- "family", "hierarchy", "username"
+ "family", "hierarchy", "username", "output"
  ]
  skip_host_families = []
 
@@ -517,20 +511,22 @@ def prepare_representation(self, repre,
 
  # pre-flight validations
  if repre["ext"].startswith("."):
- raise ValueError("Extension must not start with a dot '.': "
- "{}".format(repre["ext"]))
+ raise KnownPublishError((
+ "Extension must not start with a dot '.': {}"
+ ).format(repre["ext"]))
 
  if repre.get("transfers"):
- raise ValueError("Representation is not allowed to have transfers"
- "data before integration. They are computed in "
- "the integrator"
- "Got: {}".format(repre["transfers"]))
+ raise KnownPublishError((
+ "Representation is not allowed to have transfers"
+ "data before integration. They are computed in "
+ "the integrator. Got: {}"
+ ).format(repre["transfers"]))
 
  # create template data for Anatomy
  template_data = copy.deepcopy(instance.data["anatomyData"])
 
  # required representation keys
- files = repre['files']
+ files = repre["files"]
  template_data["representation"] = repre["name"]
  template_data["ext"] = repre["ext"]
 
@@ -546,68 +542,68 @@ def prepare_representation(self, repre,
  }.items():
  # Allow to take value from representation
  # if not found also consider instance.data
- if key in repre:
- value = repre[key]
- elif key in instance.data:
- value = instance.data[key]
- else:
- continue
- template_data[anatomy_key] = value
+ value = repre.get(key)
+ if value is None:
+ value = instance.data.get(key)
 
- if repre.get('stagingDir'):
- stagingdir = repre['stagingDir']
- else:
+ if value is not None:
+ template_data[anatomy_key] = value
+
+ stagingdir = repre.get("stagingDir")
+ if not stagingdir:
  # Fall back to instance staging dir if not explicitly
  # set for representation in the instance
- self.log.debug("Representation uses instance staging dir: "
- "{}".format(instance_stagingdir))
+ self.log.debug((
+ "Representation uses instance staging dir: {}"
+ ).format(instance_stagingdir))
  stagingdir = instance_stagingdir
+
  if not stagingdir:
- raise ValueError("No staging directory set for representation: "
- "{}".format(repre))
+ raise KnownPublishError(
+ "No staging directory set for representation: {}".format(repre)
+ )
 
  self.log.debug("Anatomy template name: {}".format(template_name))
- anatomy = instance.context.data['anatomy']
- template = os.path.normpath(anatomy.templates[template_name]["path"])
+ anatomy = instance.context.data["anatomy"]
+ publish_template_category = anatomy.templates[template_name]
+ template = os.path.normpath(publish_template_category["path"])
 
  is_udim = bool(repre.get("udim"))
+
  is_sequence_representation = isinstance(files, (list, tuple))
  if is_sequence_representation:
  # Collection of files (sequence)
- assert not any(os.path.isabs(fname) for fname in files), (
- "Given file names contain full paths"
- )
+ if any(os.path.isabs(fname) for fname in files):
+ raise KnownPublishError("Given file names contain full paths")
 
  src_collection = assemble(files)
 
- # If the representation has `frameStart` set it renumbers the
- # frame indices of the published collection. It will start from
- # that `frameStart` index instead. Thus if that frame start
- # differs from the collection we want to shift the destination
- # frame indices from the source collection.
  destination_indexes = list(src_collection.indexes)
- destination_padding = len(get_first_frame_padded(src_collection))
- if repre.get("frameStart") is not None and not is_udim:
- index_frame_start = int(repre.get("frameStart"))
-
- render_template = anatomy.templates[template_name]
- # todo: should we ALWAYS manage the frame padding even when not
- # having `frameStart` set?
- frame_start_padding = int(
- render_template.get(
- "frame_padding",
- render_template.get("padding")
- )
+ # Use last frame for minimum padding
+ # - that should cover both 'udim' and 'frame' minimum padding
+ destination_padding = len(str(destination_indexes[-1]))
+ if not is_udim:
+ # Change padding for frames if template has defined higher
+ # padding.
+ template_padding = int(
+ publish_template_category["frame_padding"]
  )
-
- # Shift destination sequence to the start frame
- src_start_frame = next(iter(src_collection.indexes))
- shift = index_frame_start - src_start_frame
- if shift:
+ if template_padding > destination_padding:
+ destination_padding = template_padding
+
+ # If the representation has `frameStart` set it renumbers the
+ # frame indices of the published collection. It will start from
+ # that `frameStart` index instead. Thus if that frame start
+ # differs from the collection we want to shift the destination
+ # frame indices from the source collection.
+ repre_frame_start = repre.get("frameStart")
+ if repre_frame_start is not None:
+ index_frame_start = int(repre["frameStart"])
+ # Shift destination sequence to the start frame
  destination_indexes = [
- frame + shift for frame in destination_indexes
+ index_frame_start + idx
+ for idx in range(len(destination_indexes))
  ]
- destination_padding = frame_start_padding
 
  # To construct the destination template with anatomy we require
  # a Frame or UDIM tile set for the template data. We use the first
@@ -625,16 +621,25 @@ def prepare_representation(self, repre,
  anatomy_filled = anatomy.format(template_data)
  template_filled = anatomy_filled[template_name]["path"]
  repre_context = template_filled.used_values
+
+ # Make sure context contains frame
+ # NOTE: Frame would not be available only if template does not
+ # contain '{frame}' in template -> Do we want support it?
+ if not is_udim:
+ repre_context["frame"] = first_index_padded
+
  self.log.debug("Template filled: {}".format(str(template_filled)))
  dst_collection = assemble([os.path.normpath(template_filled)])
 
  # Update the destination indexes and padding
  dst_collection.indexes.clear()
  dst_collection.indexes.update(set(destination_indexes))
  dst_collection.padding = destination_padding
- assert (
- len(src_collection.indexes) == len(dst_collection.indexes)
- ), "This is a bug"
+ if len(src_collection.indexes) != len(dst_collection.indexes):
+ raise KnownPublishError((
+ "This is a bug. Source sequence frames length"
+ " does not match integration frames length"
+ ))
 
  # Multiple file transfers
  transfers = []
@@ -645,9 +650,13 @@ def prepare_representation(self, repre,
  else:
  # Single file
  fname = files
- assert not os.path.isabs(fname), (
- "Given file name is a full path"
- )
+ if os.path.isabs(fname):
+ self.log.error(
+ "Filename in representation is filepath {}".format(fname)
+ )
+ raise KnownPublishError(
+ "This is a bug. Representation file name is full path"
+ )
 
  # Manage anatomy template data
  template_data.pop("frame", None)
@@ -677,9 +686,8 @@ def prepare_representation(self, repre,
  # Also add these values to the context even if not used by the
  # destination template
  value = template_data.get(key)
- if not value:
- continue
- repre_context[key] = template_data[key]
+ if value is not None:
+ repre_context[key] = value
 
  # Explicitly store the full list even though template data might
  # have a different value because it uses just a single udim tile
@@ -693,40 +701,30 @@ def prepare_representation(self, repre,
  else:
  repre_id = ObjectId()
 
- # Backwards compatibility:
  # Store first transferred destination as published path data
- # todo: can we remove this?
- # todo: We shouldn't change data that makes its way back into
- # instance.data[] until we know the publish actually succeeded
- # otherwise `published_path` might not actually be valid?
+ # - used primarily for reviews that are integrated to custom modules
+ # TODO we should probably store all integrated files
+ # related to the representation?
  published_path = transfers[0][1]
- repre["published_path"] = published_path # Backwards compatibility
+ repre["published_path"] = published_path
 
  # todo: `repre` is not the actual `representation` entity
  # we should simplify/clarify difference between data above
  # and the actual representation entity for the database
  data = repre.get("data", {})
- data.update({'path': published_path, 'template': template})
+ data.update({"path": published_path, "template": template})
  representation = {
  "_id": repre_id,
  "schema": "openpype:representation-2.0",
  "type": "representation",
  "parent": version["_id"],
- "name": repre['name'],
+ "name": repre["name"],
  "data": data,
 
  # Imprint shortcut to context for performance reasons.
  "context": repre_context
  }
 
- # todo: simplify/streamline which additional data makes its way into
- # the representation context
- if repre.get("outputName"):
- representation["context"]["output"] = repre['outputName']
-
- if is_sequence_representation and repre.get("frameStart") is not None:
- representation['context']['frame'] = template_data["frame"]
-
  return {
  "representation": representation,
  "anatomy_data": template_data,
@@ -786,7 +784,7 @@ def create_version_data(self, instance):
  version_data[key] = instance.data[key]
 
  # Include instance.data[versionData] directly
- version_data_instance = instance.data.get('versionData')
+ version_data_instance = instance.data.get("versionData")
  if version_data_instance:
  version_data.update(version_data_instance)
 
@@ -826,6 +824,7 @@ def _get_template_name_profiles(self, instance):
 
  def get_profile_filter_criteria(self, instance):
  """Return filter criteria for `filter_profiles`"""
+
  # Anatomy data is pre-filled by Collectors
  anatomy_data = instance.data["anatomyData"]
 
@@ -856,6 +855,7 @@ def get_rootless_path(self, anatomy, path):
  path: modified path if possible, or unmodified path
  + warning logged
  """
+
  success, rootless_path = anatomy.find_root_template_from_path(path)
  if success:
  path = rootless_path
@@ -877,6 +877,7 @@ def get_files_info(self, destinations, sites, anatomy):
  output_resources: array of dictionaries to be added to 'files' key
  in representation
  """
+
  file_infos = []
  for file_path in destinations:
  file_info = self.prepare_file_info(file_path, anatomy, sites=sites)
@@ -896,10 +897,11 @@ def prepare_file_info(self, path, anatomy, sites):
  Returns:
  dict: file info dictionary
  """
+
  return {
  "_id": ObjectId(),
  "path": self.get_rootless_path(anatomy, path),
  "size": os.path.getsize(path),
- "hash": openpype.api.source_hash(path),
+ "hash": source_hash(path),
  "sites": sites
  }