Dump out correlation id without verbose logging for resource deployment#4532
Dump out correlation id without verbose logging for resource deployment#45321 commit merged intoAzure:mainfrom
Conversation
benbp
added
the
Central-EngSys
|
The following pipelines have been queued for testing: |
| New-AzResourceGroupDeployment -Name $BaseName -ResourceGroupName $resourceGroup.ResourceGroupName -TemplateFile $templateFile.jsonFilePath -TemplateParameterObject $templateFileParameters -Force:$Force | ||
| } catch { | ||
| Write-Output @' | ||
| New-AzResourceGroupDeployment ` |
There was a problem hiding this comment.
Based on the old code it would seem as though this might throw exceptions if it failed. Do you know if that is true or not?
There was a problem hiding this comment.
Yes, it could throw or return an error- try...catch...finally in powershell works for both. The idea was to reset the $DebugPreference, which we don't need anymore. Why it kept going I don't know - might've been an oversight after all the changes in this area over the years.
There was a problem hiding this comment.
It does throw still, which gets caught by the Retry function handler instead. I was confused at first as well (though it was my old code so...)
There was a problem hiding this comment.
I guess Retry only exits with a Write-Error, so perhaps we should actually re-throw in case the ErrorActionPreference is different for local users?
There was a problem hiding this comment.
That could work.
There are some ill-behaved cmdlets like this that actual throw instead of write error, including the first-party Invoke-WebRequest (which just bit me today; I had no idea it wasn't...proper).
| -Force:$Force | ||
| } | ||
|
|
||
| if ($deployment.ProvisioningState -ne 'Succeeded') { |
There was a problem hiding this comment.
Hopefully explicitly checking for succeeded will help handle some error cases that were slipping through.
There was a problem hiding this comment.
Yeah we weren't handling non-succeeded states any differently before. IIRC we saw some issues with the state being Incremental or something like that.
weshaggard
left a comment
weshaggard
left a comment
There was a problem hiding this comment.
Other than the one question about exceptions this looks good.
| New-AzResourceGroupDeployment -Name $BaseName -ResourceGroupName $resourceGroup.ResourceGroupName -TemplateFile $templateFile.jsonFilePath -TemplateParameterObject $templateFileParameters -Force:$Force | ||
| } catch { | ||
| Write-Output @' | ||
| New-AzResourceGroupDeployment ` |
There was a problem hiding this comment.
Yes, it could throw or return an error- try...catch...finally in powershell works for both. The idea was to reset the $DebugPreference, which we don't need anymore. Why it kept going I don't know - might've been an oversight after all the changes in this area over the years.
|
Hello @azure-sdk! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 4 participants
We don't really need this verbose logging anymore. Removing it to save log space, as well as mitigate potential issues in the NET storage pipelines where the log output gets truncated, meaning pipeline variables don't get set.