Skip to content

Fixes for Azure Automation module #7

Merged
safeermohammed merged 2 commits intosafeermohammed:previewfrom
Francisco-Gamino:AzureAutomation-fixes
Aug 21, 2018
Merged

Fixes for Azure Automation module #7
safeermohammed merged 2 commits intosafeermohammed:previewfrom
Francisco-Gamino:AzureAutomation-fixes

Conversation

@Francisco-Gamino
Copy link
Copy Markdown
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
Copy link
Copy Markdown

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
Copy Markdown
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
Copy link
Copy Markdown

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
Copy Markdown
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.


private string ParseErrorMessage(string errorMessage)
// This function determines if the given value is a Json object.
private bool IsValidJsonObject(string value)
Copy link
Copy Markdown

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
Copy Markdown
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