-
Notifications
You must be signed in to change notification settings - Fork 67
Implement ValidateFolderCreationResolution. #1293
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
base: develop
Are you sure you want to change the base?
Changes from all commits
9ab1b9e
eca282e
3581a7e
92457a9
5f1231e
588371d
637adde
7907015
93f4e89
e2650ad
aa65478
345ae77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,17 @@ | ||
| import pyblish.api | ||
| from ayon_core.pipeline import PublishValidationError | ||
|
|
||
| from ayon_api.entity_hub import EntityHub | ||
|
|
||
| from ayon_core.lib import BoolDef | ||
| from ayon_core.pipeline.publish import ( | ||
| PublishValidationError, | ||
| RepairAction, | ||
| get_errored_instances_from_context, | ||
| AYONPyblishPluginMixin | ||
| ) | ||
|
|
||
|
|
||
| _RESOLUTION_ATTRIBS = ("resolutionHeight", "resolutionWidth", "pixelAspect") | ||
|
|
||
|
|
||
| class ValidateFolderEntities(pyblish.api.InstancePlugin): | ||
|
|
@@ -37,3 +49,121 @@ def process(self, instance): | |
| "Instance \"{}\" doesn't have folder entity " | ||
| "set which is needed for publishing." | ||
| ).format(instance.data["name"])) | ||
|
|
||
|
|
||
| class RepairIgnoreResolution(RepairAction): | ||
| """ Repair, disable resolution update in problematic instance(s). | ||
| """ | ||
| label = "Skip folder resolution validation." | ||
|
|
||
| def process(self, context, plugin): | ||
| create_context = context.data["create_context"] | ||
| for inst in get_errored_instances_from_context(context, plugin=plugin): | ||
| instance_id = inst.data.get("instance_id") | ||
| created_instance = create_context.get_instance_by_id(instance_id) | ||
| attr_values = created_instance.data["publish_attributes"].get( | ||
| "ValidateFolderCreationResolution", {}) | ||
| attr_values["validateExistingFolderResolution"] = False | ||
|
|
||
| create_context.save_changes() | ||
|
|
||
|
|
||
| class ValidateFolderCreationResolution( | ||
| pyblish.api.InstancePlugin, | ||
| AYONPyblishPluginMixin | ||
| ): | ||
| """ Validate resolution values before updating an existing folder. | ||
| """ | ||
|
|
||
| label = "Validate new folder resolution" | ||
| order = pyblish.api.ValidatorOrder | ||
| families = ["shot"] | ||
| actions = [RepairIgnoreResolution] | ||
|
|
||
| def _validate_hierarchy_resolution(self, hierarchy_context): | ||
| """ Deep validation of hierarchy_context resolution. | ||
| """ | ||
| project_name = tuple(hierarchy_context.keys())[0] | ||
| entity_hub = EntityHub(project_name) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion to avoid using EntityHub here. It would be faster to prepare all folder paths you need, fetch all folder entities at once using those paths and then compare them. entity_items_by_path = {}
folder_paths = []
current_item = next(iter(hierarchy_context.values()))
queue = collections.deque()
queue.append((current_item, ""))
while queue:
current_item, current_path = queue.popleft()
children = current_item.get("children") or {}
for name, child in children.items():
path = f"{current_path}/{name}"
folder_paths.append(path)
queue.append((child, path))
# Skip validation of items without attributes
if child.get("attributes"):
entity_items_by_path[path] = child
# Fetch folder entities that should be validated
folder_entities_by_path = {
folder_entity["path"]: folder_entity
for folder_entity in ayon_api.get_folders(
project_name,
folder_paths=folder_paths,
fields={"path", "attrib"}
)
}
# Run the validation
for path, item in entity_items_by_path.items():
folder_entity = folder_entities_by_path.get(path)
if folder_entity:
self._validate_folder_resolution(
item["attributes"],
folder_entity,
)(Would require change in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think this should be addressed. Using EntityHub can slow down the publishing a lot. EntityHub is fetching the entities one by one if are not pre-fetched... You know ahead which entities you're interested in, so using If you'd compare e.g. 1 seqeunce vs. 10 sequences it can be 10x slower.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rdelillo are you planning to address the changes and remove the EntityHub? |
||
| entity_data = {project_name: entity_hub.project_entity} | ||
| entity_to_inspect = [(entity_data, hierarchy_context)] | ||
|
|
||
| while entity_to_inspect: | ||
| entity_data, data = entity_to_inspect.pop(0) | ||
|
|
||
| for name, value in data.items(): | ||
| entity = entity_data.get(name) | ||
| child = value.get("children", None) | ||
|
|
||
| # entity exists in AYON. | ||
| if entity: | ||
| folder_data = value.get("attributes", {}) | ||
| self._validate_folder_resolution( | ||
| folder_data, | ||
| entity, | ||
| ) | ||
|
|
||
| if child: | ||
| entity_children = { | ||
| chld.name: chld | ||
| for chld in entity.children | ||
| } | ||
| entity_to_inspect.append((entity_children, child)) | ||
|
|
||
| def _validate_folder_resolution(self, folder_data, entity): | ||
| """ Validate folder resolution against existing data. | ||
| """ | ||
| similar = True | ||
| for resolution_attrib in _RESOLUTION_ATTRIBS: | ||
| folder_value = folder_data.get(resolution_attrib) | ||
| entity_value = entity.attribs.get(resolution_attrib) | ||
| if folder_value and folder_value != entity_value: | ||
| self.log.warning( | ||
| f"Resolution mismatch for folder {entity.name}. " | ||
| f"{resolution_attrib}={folder_value} but " | ||
| f" existing entity is set to {entity_value}." | ||
| ) | ||
| similar = False | ||
|
|
||
| if not similar: | ||
| resolution_data = { | ||
| key: value | ||
| for key, value in folder_data.items() | ||
| if key in _RESOLUTION_ATTRIBS | ||
| } | ||
| raise PublishValidationError( | ||
| "Resolution mismatch for " | ||
| f"folder {entity.name} (type: {entity.folder_type}) " | ||
| f"{resolution_data} does not " | ||
| "correspond to existing entity." | ||
| ) | ||
|
|
||
| def process(self, instance): | ||
| """ Validate existing folder resolution. | ||
| """ | ||
| values = self.get_attr_values_from_data(instance.data) | ||
| if not values.get("validateExistingFolderResolution", False): | ||
| self.log.debug("Skip existing folder(s) resolution validation ") | ||
| return | ||
|
|
||
| hierarchy_context = instance.context.data.get("hierarchyContext") | ||
| if not hierarchy_context: | ||
| self.log.debug("No hierarchy context defined for instance.") | ||
| return | ||
|
|
||
| self._validate_hierarchy_resolution(hierarchy_context) | ||
|
|
||
| @classmethod | ||
| def get_attr_defs_for_instance( | ||
| cls, create_context, instance, | ||
| ): | ||
| if not cls.instance_matches_plugin_families(instance): | ||
| return [] | ||
|
|
||
| return [ | ||
| BoolDef( | ||
| "validateExistingFolderResolution", | ||
| default=False, | ||
| label="Validate existing folders resolution", | ||
| ), | ||
| ] | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this plugin should be Context plugin to run only once, or?
Currently it is instance plugin, with attribute on each instance, but validates all instances (and is running on all instances).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the main reason this is on instance is that user can control this per instance @rdelillo ?