Initial trigger logs implementation #900
Conversation
| ret_val[method] = ex | ||
| else: | ||
| ret_val[method] = multi_response.get("result") |
There was a problem hiding this comment.
This changes the earlier behavior not to crash on errors, which would also fix the issue encountered in #852.
| @@ -35,6 +35,14 @@ def __init__( | |||
|
|
|||
| async def update(self, update_children: bool = True): | |||
| """Noop update. The parent updates our internals.""" | |||
| # TODO: we should probably default update_children=False in the library | |||
| # and perform these extra queries only on request. | |||
| if update_children: | |||
There was a problem hiding this comment.
The above todo is something we should discuss about, if we change the update_children=False, we could modify SmartDevice.update() to call updates on children, too?
There was a problem hiding this comment.
Is this still a todo for 0.7.0? I think that the default should probably be False and then we can always have the option on calling update on children directly when needed in the future.
| try: | ||
| self._handle_response_error_code(multi_response, method) | ||
| except KasaException as ex: | ||
| ret_val[method] = ex | ||
| else: | ||
| ret_val[method] = multi_response.get("result") |
There was a problem hiding this comment.
| try: | |
| self._handle_response_error_code(multi_response, method) | |
| except KasaException as ex: | |
| ret_val[method] = ex | |
| else: | |
| ret_val[method] = multi_response.get("result") | |
| self._handle_response_error_code(multi_response, method, raise_on_error=False) |
Wouldn't this achieve the same thing although the result will be the error code rather than the exception (which is consistent with the other handling)
| @@ -35,6 +35,14 @@ def __init__( | |||
|
|
|||
| async def update(self, update_children: bool = True): | |||
| """Noop update. The parent updates our internals.""" | |||
| # TODO: we should probably default update_children=False in the library | |||
| # and perform these extra queries only on request. | |||
| if update_children: | |||
There was a problem hiding this comment.
Is this still a todo for 0.7.0? I think that the default should probably be False and then we can always have the option on calling update on children directly when needed in the future.
|
Yes, I was about to write that we probably only want the error handling piece in as a separate PR, as it will also fix the issue when one tries to command a device with out-of-sync time. I'll separate it from the trigger log implementation & leave the update children also out, as that's something which will require some more thought and testing anyway. |
No description provided.