-
Notifications
You must be signed in to change notification settings - Fork 0
Fixes for Azure Automation module #7
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
Fixes for Azure Automation module #7
Conversation
…Schedule by setting the correct JobScheduleId; fixing exception handling for source control cmdlets; Adding logic to ExecuteCmdlet, so that when a ErrorResponseException is thrown, we check to see if ErrorResponseException.Body.Message is available. If not, we try to extracted from error message from ErrorResponseException.Response.Content, and we rethrow again using the extracted message. With this fix, the correct error message for invalid ResourceGroupName and AutomationAccountName is displayed to the user.
} | ||
|
||
/// The error information. | ||
public class AzureAutomationErrorInfo |
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.
This is not necessary if you use AzureAutomationErrorResponseMessage
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.
Good point. Fixed.
/// <summary> | ||
/// The error response message. | ||
/// </summary> | ||
public class AzureAutomationErrorResponseMessage |
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.
How about naming it AzureAutomationErrorInfo?
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.
Sounds good. Fixed.
@@ -131,29 +168,24 @@ protected bool GenerateCmdletOutput(IEnumerable<object> results) | |||
return ret; | |||
} | |||
|
|||
private string ParseErrorMessage(string errorMessage) | |||
// This function determines if the given value is a Json object. | |||
private bool IsValidJsonObject(string value) |
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.
Is this really necessary? It seems like extra work. How about just attempting to parse and handling errors?
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.
I agree. I will go ahead and remove the code that checks if it is a valid Json. In ParseJson, if we are not able to extract the error message, we just return null.
This PR contains the following fixes: