Adding status file for non enable commands and catching early errors #144
Conversation
rane-rajasi
requested review from
a team,
benank,
kjohn-msft and
mirichmo
and removed request for
a team
Codecov Report
@@ Coverage Diff @@
## master #144 +/- ##
=======================================
Coverage 87.24% 87.24%
=======================================
Files 48 48
Lines 8363 8363
=======================================
Hits 7296 7296
Misses 1067 1067
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
| def uninstall(self): | ||
| try: | ||
| self.setup(action=Constants.UNINSTALL, log_message="Extension uninstalled") | ||
| self.ext_output_status_handler.write_status_file("", self.seq_no, status=Constants.Status.Success.lower()) |
There was a problem hiding this comment.
Please check with Jemal about the cases where the Windows extension writes a status file to ensure alignment between the extensions.
There was a problem hiding this comment.
Done, windows extension also writes status only for enable. Windows agent may write status file for enable, this is considered by our extension while writing status. As for other handler actions, we aren't sure if agent does write status and the extension does not write status
can we add formatted json string, that has error code and if the error code is client error or platform error? |
Based on discussion, decided to not include this change, since we don't need additional error information at this point of time. |
| except Exception: | ||
| raise | ||
| finally: | ||
| self.write_basic_status(action) |
There was a problem hiding this comment.
curiosity: If lines 69 through 72 throw, can the exception be written to the basic status file and then rethrown?
It looks like you'll have to write basic status on line 73 at the end of the try and within the except. In that case a finally will no longer be necessary.
There was a problem hiding this comment.
With basic status, we set the top level status to "transitioning". Now incase of exceptions, and all these are currently terminal, the status is re-written to "error"
The way the code is in all setup() callers right now, we can have write_basic_status() on line 72, just before except, so a basic status (with status=Transitioning) is written in a happy path, and incase of exceptions, there is only 1 status write with status=error (instead of 2 writes). But thought of keeping write_basic_status() in finally, in case something changes in callers later
Changes included:
[{
"version": 1.0,
"timestampUTC": "2019-07-20T12:12:14Z",
"status": {
"name": "Azure Patch Management",
"operation": "",
"status": "Error",
"code": <error_code>,
"formattedMessage": {
"lang": "en-US",
"message": ""
}
}
}]