partial hierarchy with asset linked to existing asset by parent_id
Closed this issue · 4 comments
System information (please complete the following information):
- OS: Ubuntu
- Python Version: python3.10
- SDK Version: 6.6.0
Describe the bug
According to the documentation it is allowed to create “partial hierarchies” (docs), i.e which depend on existing assets.
It is said explicitly, that "asset with specified parent_id are considered valid" which is not true in the current version of sdk
To Reproduce
Runnable code reproducing the error.
from cognite.client.data_classes import (
Asset,
AssetHierarchy,
)
id_of_potentially_existing_asset = 123
asset = Asset(parent_id=id_of_potentially_existing_asset, name="new asset", external_id="test.external.id")
hierarchy = AssetHierarchy([asset])
print(hierarchy.is_valid()) # returns False
Expected behavior
should pass validation
Additional context
The piece of code below doesn’t take into account an option when “parent_id was provided (and is from existing asset, so not part of the partial hierarchy) and parent_external_id was not provided”
if (pxid := asset.parent_external_id) is (pid := asset.parent_id) is None:
roots.append(asset)
elif pxid is not None and pid is not None:
unsure_parents.append(asset) # Both parent links are given
elif pxid not in xid_count:
orphans.append(asset) # Only parent XID given, but not part of assets given
Thanks for the bug report, will look into it asap!
@vindex10 So, the thing is that the asset hierarchy validation code used to classify "all orphans" as orphans, regardless of whether they linked a parent through ID or external ID. But as you point out, this does not make sense for the "parent ID linked assets" as the given assets can never have ID set and so we can never validate these anyway and should just take it for granted that they are valid - this is also what the documentation says.
I've made a PR (#1283 ) that changes this; only "parent XID linked assets" can ever be put into the "orphans category". Please review it to check that it is aligned with your expectations 😄
Fix available as of >=6.8.1
Used it already, works fine. Thank you!