gergo/applicationIdAttach #391
Conversation
| @@ -332,7 +332,7 @@ def _mark_run( | |||
| def attach_error_to_objects( | |||
| self, | |||
| category: str, | |||
| object_ids: Union[str, List[str]], | |||
| objects: Union[Base, List[Base]], | |||
There was a problem hiding this comment.
If we are changing this anyway, I think requiring a List[Base] only is preferable to either a single or a list
| raise ValueError( | ||
| f"Need atleast one object_id to report a(n) {level.value.upper()}" | ||
| ) | ||
| id_list = object_ids | ||
| id_list = [o.id for o in objects] |
There was a problem hiding this comment.
im not a fan of this - partly because we are storing a list of objects as result data contextual to the objects - ids worked well here as there wasn't an inference the developer could make that this could be NEW Base objects.
We don't serialize these objects in the same way as a Send, do we???
There was a problem hiding this comment.
No we do not, good point. We should make sure, that the object is part of the original root object hierarchy.
There was a problem hiding this comment.
Are we hoping that these are Base objects that have been received by the automation script?
If so, they should always have an ID, but we should handle the case where the user passes in a Base object that doesn't have an ID. Probably by throwing since that object would not be part of the automation trigger commit.
|
closing in favor of #424 |
Adds application ids to the result metadata