Output Error Fixes for ADDS#255
Open
vmarcelli-eng wants to merge 81 commits intoAzure-Samples:masterfrom
Open
Conversation
If the error is not expected, change the print from "Failed" to "Unexpected"
added updated messages for pass and for fail
SummaryOfChecks does the same thing at the following code block replaced
MaximeKjaer
reviewed
Feb 11, 2025
| + "`n`tTo debug connectivity problems, please refer to the following," ` | ||
| + "`n`t'AzFileDiagnostics.ps1'($($PSStyle.Foreground.BrightCyan)https://github.com/Azure-Samples/azure-files-samples/tree/master/AzFileDiagnostics/Windows$($PSStyle.Reset))'." ` | ||
| + "`n`tFor possible solutions please refer to '$($PSStyle.Foreground.BrightCyan)https://aka.ms/azfiles/entra-port445$($PSStyle.Reset)'" | ||
| Write-TestingFailed -Message $errMsg -ErrorAction Stop |
Collaborator
There was a problem hiding this comment.
I don't think the -ErrorAction Stop is doing anything here, we can remove it. To test this, you can try:
try { Write-Error "test" -ErrorAction Stop } catch { Write-Host "caught write-error" }
try { Write-TestingFailed "test" -ErrorAction Stop } catch { Write-Host "caught write-testingfailed" }
| $checks["CheckDomainLineOfSight"].Issue = $_ | ||
| Write-Error "CheckDomainLineOfSight - FAILED" | ||
| Write-Error $_ | ||
| Write-TestingFailed -Message $_ -IsUnexpected $true |
Collaborator
There was a problem hiding this comment.
I think we may need to make some edits to Debug-DomainLineOfSight to also print in the expected way. Maybe we can make it return a bool saying whether we have line-of-sight or not, and then print what we need to here?
| $checks["CheckADObjectPasswordIsCorrect"].Issue = $_ | ||
| Write-Error "CheckADObjectPasswordIsCorrect - FAILED" | ||
| Write-Error $_ | ||
| Write-TestingFailed -Mesage $_ -IsUnexpected $true |
Collaborator
There was a problem hiding this comment.
Same thing here, I think it's not necessarily unexpected. This is because Test-AzStorageAccountADObjectPasswordIsKerbKey throws an error when it falis. But I don't love that pattern, doesn't let us distinguish test failures from actual exceptional cases that warrant exceptions.
Co-authored-by: Maxime Kjaer <maxime.kjaer@gmail.com>
updated aka.ms link to https://learn.microsoft.com/en-us/troubleshoot/azure/azure-storage/files/connectivity/files-troubleshoot?tabs=powershell#check-tcp-connectivity Co-authored-by: Maxime Kjaer <maxime.kjaer@gmail.com>
Co-authored-by: Maxime Kjaer <maxime.kjaer@gmail.com>
Co-authored-by: Maxime Kjaer <maxime.kjaer@gmail.com>
removed verbose messages that were replaced by new output. Messages which were not replaced were not removed.
replaced with new output
replaced with new output
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updated output error messages for Debug-AzStorageAccountADDSAuth