xWindowsOptionalFeature: Suppress useless verbose output #454
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #454 +/- ##
===================================
Coverage 72% 72%
===================================
Files 27 27
Lines 4031 4031
Branches 4 4
===================================
Hits 2922 2922
Misses 1105 1105
Partials 4 4 |
|
Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again. |
|
Question for @mkht , and for the community at large. Can we envision any scenarios where someone would actually want to see this verbose Import-Module output? Would it ever be useful for troubleshooting (like if the Import failed for some reason)? If so, this may be better implemented as a configurable parameter to the module, not as a change that just turns it off permanently (although the new parameter could default to suppressing the output). |
|
Thank you for reply @mhendric I can not imagine the situation where the output of Import-Module is useful for troubleshooting, but if we have such a scenario, we may leave a verbose message. Or how about outputting to a debug message instead of a verbose message? In any case, I do not agree to add new parameters to determine whether to output message or not. It will make more complicated than necessary and I do not know another DSC resources that have such parameters. |
|
Hi @mkht , the Debug option may be a good option if it's possible. Not sure if this will work, but you could try adding -Debug:$DebugPreference to the Import-Module command, and then confirm what it outputs if you pass the -Debug switch to Start-DscConfiguration. Also confirm whether it adds all the output you're trying to suppress, so that the output is still available if someone needs it. |
@mhendric Seems that other DSC resources (e.g. xWindowsFeature) have the same issue. Ref to the verbose output attached to issue #452 for an example. |
|
I'm sorry. I have no time to deal with this issue. Please wait for a while. |
|
I just tested, and using -Debug:$DebugPreference does not add the output lost by setting -Verbose:$false. @PlagueHO , can you weigh in on whether you think we should leave this Verbose Import-Module output or take it out? I'm fine either way. |
|
I don't recall a situation where I've found the verbose output of Import-Module useful and anytime I was trying to diagnose module load issues I was already getting an exception thrown from the module import. So the verbose logs didn't help. That said, I generally don't apply DSC configs with Verbose enabled. I only enable it when I'm debugging something. I'm not adverse to using @johlju - from memory SqlServerDsc uses Import-Module - what do you do over there? |
PlagueHO
added
the
waiting for author response
|
I will immediately update the document and prepare to merge it into the dev branch. For your reference, # Check that Dism PowerShell module is available
Import-Module -Name 'Dism' -ErrorVariable 'errorsFromDismImport' -ErrorAction 'SilentlyContinue' -Force -Verbose 4>&1 | Write-Debug |
|
In the SqlServerDsc module we have a helper function ( |
|
I think what @johlju suggested (minus the helper function) is what @mkht already did with this submission (set -Verbose:$false). If we're all in agreement that that's the right approach, I think we just need to resolve conflicts and call it good. Ultimately I could still go either way on this, but don't think it's worth spending a ton of time debating this one. I don't necessarily like removing logging, but I agree that this particular logging is probably never gonna be useful (and if there are issues, they will probably show up as a Warning or Error). |
|
Also, looks like this one somehow ended up with a double Readme entry (one under Unreleased and one under 8.5.0.0). We should probably correct that. |
|
So I guess we go with what @mkht said from the get-go 😁 . Yep, better fix the readme - good catch! |
mhendric
removed
the
waiting for author response
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 1 of 1 files at r5.
Reviewable status:complete! all files reviewed, all discussions resolved
There was a problem hiding this comment.
Reviewed 1 of 1 files at r6.
Reviewable status:
Pull Request (PR) description
Suppress annoying verbose output of xWindowsOptionalFeature
This Pull Request (PR) fixes the following issues
Fixes #453
Task list
Entry should say what was changed, and how that affects users (if applicable).
and comment-based help.
This change is