Global: Add version number to subset uniqueness validation

[jakubjezek001]

Changelog Description

This changes improves the accuracy of subset uniqueness validation by including the version number in the check.

Additional info

This change adds the version number to the validation of unique subsets in the ValidateSubsetUniqueness class. Previously, it only checked for overlapping instances based on asset and subset names. Now, it also considers the version number when determining uniqueness. If multiple versions under the same subset are published, a warning is logged and those instances are skipped during validation.

[ynbot]

Task linked: OP-6539 Traypublisher: advance editorial publishing from CSV

[BigRoy]

This will easily produce a cluttered artist facing log for which the artist knows nothing about what to do with this. It's not an actual issue - this is totally valid as far as I know. I would not log this actually. It's not a special case, also the check below in else: is also completely valid warbubg for the case where multiple instances with matching subset and asset include one that is None (actually, one might argue that if one has None and the others do not and their version numbers are higher than the current last version that the behavior is still dangerous and may be considered invalid/erroneous)

[BigRoy]

I wouldn't do this check in this for loop - it's basically much slower like this because you iterate over all per entry.

[BigRoy]

Likely you're better off doing something like:

instances_per_asset_subset[asset, subset].append(instance)

And then:

for (asset, subset), instances in instances_per_asset_subset.items():
    
    if len(instances) < 2:
        # All good, ignore
        continue

    # An instance *may* explicitly set a specific version to publish into
    # and if so, it is allowed to publish into the same subset if the versions
    # differ between the instances. So we detect whether the instances
    # are set to publish to a different version
    instances_by_version = defaultdict(list)
    for instance in instances:
        version = instance.data.get("version")
        instances_by_version[version].append(instance)

    multiple_instances_per_version = False
    for version, version_instances in instances_by_version.items():
        if len(version_instances) > 2:
            # This is not allowed - multiple instances targeting same version
            multiple_instances_per_version = True
            non_unique.append(
                "{asset} > {subset} > {version}".format(
                    asset=asset, subset=subset, version=version
                )
            )

    if not multiple_instances_per_version:
        versions = ", ".join(str(version) for version in instances_by_version)
        self.log.warning(
            "Multiple instances are writing to '%s' > '%s' but to different versions. "
            "This is allowed but is considered an edge case. "
            "The versions to be published are: %s",
            asset, subset, versions
        )
   

So that the warning log only shows if there isn't already an error anyway.

[tokejepsen]

Can we address BigRoy's feedback before any testing?