feat(ingestion/dbt): multiple node owner separated by comma #9740
Conversation
github-actions
bot
added
the
ingestion
| @@ -278,30 +302,48 @@ def get_operation_value( | |||
| and operation_config[Constants.OWNER_TYPE] | |||
| ): | |||
| owner_id = _get_best_match(match, "owner") | |||
| owner_ids: List[str] = [] | |||
|
|
|||
| if "," in owner_id: | |||
There was a problem hiding this comment.
we don't need the if statement here
| "category": owner_category, | ||
| "categoryUrn": owner_category_urn, | ||
| } | ||
| sanitize_owner_ids = [ |
There was a problem hiding this comment.
why don't we just set owner_ids directly? do we need sanitize_owner_ids?
There was a problem hiding this comment.
used it directly. I used a separate one for readability.
| operation_map[Constants.ADD_OWNER_OPERATION], | ||
| key=lambda x: dict(x)["urn"], | ||
| ): | ||
| dict_owner_class = dict(x) |
There was a problem hiding this comment.
why is this necessary? shouldn't x already be a dict?
There was a problem hiding this comment.
operation_map[Constants.ADD_OWNER_OPERATION] is pointing to set so we do need to convert dict into tuple before storing it, hence x is tuple
There was a problem hiding this comment.
added extra if condition, now it is dict only
| @@ -160,7 +178,7 @@ def process(self, raw_props: Mapping[str, Any]) -> Dict[str, Any]: | |||
| operation_type, set() | |||
There was a problem hiding this comment.
let's add a special case for ownership, above the if isinstance(operation, (str, list)): line
operation_map[owner operation] should be a list, not a set.
hsheth2
added
the
merge-pending-ci
No description provided.