-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Service Bus : fix for issues 5058 and 5055 #6911
Conversation
@MiYanni , the CI failed with message 'Starting test execution, please wait... The job exceeded the maximum time limit for jobs, and has been terminated.' can you please restart the CI. |
@v-Ajnava You'll need to merge in latest preview into your fork. We've removed some tests from NetCore so the Travis CI process can pass. Right now, I don't think you'll have those changes. |
@v-Ajnava It seems I'm incorrect. Your branch is already up to date with preview. Let me investigate. |
@v-Ajnava I got it working. The issue was not related to your PR. |
@MiYanni Thanks ! |
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.
@v-Ajnava one comment, otherwise LGTM
public static ErrorRecord WriteErrorforBadrequest(ErrorResponseException ex) | ||
{ | ||
ErrorResponseContent errorExtract = new ErrorResponseContent(); | ||
errorExtract = JsonConvert.DeserializeObject<ErrorResponseContent>(ex.Response.Content); |
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.
@v-Ajnava can we do some null-checking in this function to ensure that ex
, ex.Response
, and ex.Response.Content
have a 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.
resolved
…nto preview_local # Conflicts: # tools/CredScanSuppressions.json
…ershell into preview_local # Conflicts: # tools/CredScanSuppressions.json
|
||
public static ErrorRecord WriteErrorforBadrequest(ErrorResponseException ex) | ||
{ | ||
if (ex != null && !string.IsNullOrEmpty(ex.Response.Content)) |
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 Response guaranteed to be non-null?
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.
yes, response will not be null
Description
Checklist
CONTRIBUTING.md
platyPS
module