-
Notifications
You must be signed in to change notification settings - Fork 41
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
gergo/applicationIdAttach #391
base: v3-dev
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -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]], | ||
message: Optional[str] = None, | ||
metadata: Optional[Dict[str, Any]] = None, | ||
visual_overrides: Optional[Dict[str, Any]] = None, | ||
|
@@ -351,7 +351,7 @@ def attach_error_to_objects( | |
self.attach_result_to_objects( | ||
ObjectResultLevel.ERROR, | ||
category, | ||
object_ids, | ||
objects, | ||
message, | ||
metadata, | ||
visual_overrides, | ||
|
@@ -360,7 +360,7 @@ def attach_error_to_objects( | |
def attach_warning_to_objects( | ||
self, | ||
category: str, | ||
object_ids: Union[str, List[str]], | ||
objects: Union[Base, List[Base]], | ||
message: Optional[str] = None, | ||
metadata: Optional[Dict[str, Any]] = None, | ||
visual_overrides: Optional[Dict[str, Any]] = None, | ||
|
@@ -369,7 +369,7 @@ def attach_warning_to_objects( | |
self.attach_result_to_objects( | ||
ObjectResultLevel.WARNING, | ||
category, | ||
object_ids, | ||
objects, | ||
message, | ||
metadata, | ||
visual_overrides, | ||
|
@@ -378,7 +378,7 @@ def attach_warning_to_objects( | |
def attach_success_to_objects( | ||
self, | ||
category: str, | ||
object_ids: Union[str, List[str]], | ||
objects: Union[Base, List[Base]], | ||
message: Optional[str] = None, | ||
metadata: Optional[Dict[str, Any]] = None, | ||
visual_overrides: Optional[Dict[str, Any]] = None, | ||
|
@@ -387,7 +387,7 @@ def attach_success_to_objects( | |
self.attach_result_to_objects( | ||
ObjectResultLevel.SUCCESS, | ||
category, | ||
object_ids, | ||
objects, | ||
message, | ||
metadata, | ||
visual_overrides, | ||
|
@@ -396,7 +396,7 @@ def attach_success_to_objects( | |
def attach_info_to_objects( | ||
self, | ||
category: str, | ||
object_ids: Union[str, List[str]], | ||
objects: Union[Base, List[Base]], | ||
message: Optional[str] = None, | ||
metadata: Optional[Dict[str, Any]] = None, | ||
visual_overrides: Optional[Dict[str, Any]] = None, | ||
|
@@ -405,7 +405,7 @@ def attach_info_to_objects( | |
self.attach_result_to_objects( | ||
ObjectResultLevel.INFO, | ||
category, | ||
object_ids, | ||
objects, | ||
message, | ||
metadata, | ||
visual_overrides, | ||
|
@@ -415,19 +415,22 @@ def attach_result_to_objects( | |
self, | ||
level: ObjectResultLevel, | ||
category: str, | ||
object_ids: Union[str, List[str]], | ||
objects: Union[Base, List[Base]], | ||
message: Optional[str] = None, | ||
metadata: Optional[Dict[str, Any]] = None, | ||
visual_overrides: Optional[Dict[str, Any]] = None, | ||
) -> None: | ||
if isinstance(object_ids, list): | ||
if len(object_ids) < 1: | ||
if isinstance(objects, list): | ||
if len(objects) < 1: | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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??? |
||
application_ids = [o.applicationId for o in objects] | ||
else: | ||
id_list = [object_ids] | ||
id_list = [objects.id] | ||
application_ids = [objects.applicationId] | ||
metadata["applicationIds"] = application_ids | ||
print( | ||
f"Created new {level.value.upper()}" | ||
f" category: {category} caused by: {message}" | ||
|
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.
If we are changing this anyway, I think requiring a List[Base] only is preferable to either a single or a list