Enhanced the functionality of SqlDacPacDeploymentonOnMachineGroupV0 #1269
Conversation
|
@microsoft-github-policy-service agree company="Microsoft" |
…entOnMachineGroupV0
| $trimmedOutput -split "`r?`n" | ForEach-Object { Write-Output $_ } | ||
| } | ||
| else { | ||
| Invoke-Expression "Invoke-SqlCmd @spaltArguments $additionalArguments" |
There was a problem hiding this comment.
Do you know if we can remove the Invoke-Expression and use directly Invoke-SqlCmd ?
Invoke-SqlCmd @invokeParams $additionalArguments
where params can probably be created by
$invokeParams = @{}
$spaltArguments.Keys | ForEach-Object {
}
There was a problem hiding this comment.
We can’t remove Invoke-Expression because $additionalArguments is a text string, not real parameters.
Invoke-SqlCmd can’t understand that text directly. It only works with proper named parameters.
Invoke-Expression is needed to run the full command as written in the string.
| $commandToRun = $commandToLog + " " + $additionalArguments | ||
| $command = "Invoke-SqlCmd @spaltArguments $additionalArguments" | ||
|
|
||
| Write-Host "##[command] $commandToRun" |
There was a problem hiding this comment.
can you make a test if additional arguments contains secret if the agent will suppress those values?
There was a problem hiding this comment.
Yeah, Razvan the agent is suppressing the value when the additional arguments contain secret, and the task is consuming its value.
|
|
||
| Write-Host "##[command] $commandToRun" | ||
|
|
||
| if ($additionalArguments.ToLower().Contains("-verbose")) { |
There was a problem hiding this comment.
don't we also have abbreviations like -v , or are there any possible arguments like -verbose:false
There was a problem hiding this comment.
No, I don't believe that's the case. I tested using both -v and -verbose:false, and we didn't receive verbose output in the logs.
|
|
||
| Write-Verbose "Invoke-SqlCmd arguments : $commandToLog $additionalArguments" | ||
| Invoke-Expression "Invoke-SqlCmd @spaltArguments $additionalArguments" | ||
| $commandToRun = $commandToLog + " " + $additionalArguments |
There was a problem hiding this comment.
Should we consider adding this entire change under FF ?
There was a problem hiding this comment.
Sure, added the entire change under the FF
| if ($additionalArguments.ToLower().Contains("-verbose")) { | ||
| $errors = @() | ||
|
|
||
| $rawOutput = (Invoke-Expression $command -ErrorVariable errors 4>&1 | Out-String) |
There was a problem hiding this comment.
wouldn't this introduce delays in providing output ? before with just invoke we would have live steaming but with this we will buffer the entire output until command finishes and after we will re-emit each line.
we will loose object types as everything is converted to plain text
There was a problem hiding this comment.
Fixed the buffering the entire output in the next commit.
and the object type "loss" is intentional and appropriate since we're displaying results to users in logs, not passing data to other PowerShell code.
Description: Added verbose log functionality while passing verbose as additional arguments
Documentation changes required: (Y/N) N
Added unit tests: (Y/N) N
Attached related issue: (Y/N) microsoft/azure-pipelines-tasks#21025