Skip to content

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

Merged

Conversation

Francisco-Gamino
Copy link
Collaborator

This PR contains the following fixes:

  • Fixing odata filter for getting a webhook by runbook name
  • Fixing JobSchedule 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.

…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

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

Copy link
Collaborator Author

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

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?

Copy link
Collaborator Author

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)

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?

Copy link
Collaborator Author

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.

@safeermohammed safeermohammed merged commit 5b4deac into safeermohammed:preview Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants