New Resource for managing choco config #132
Conversation
* development: (chocolateyGH-71) Implemented cChocoFeature (chocolateyGH-87) Moved to using reduced output (maint) formatting (chocolateyGH-70) Apply Apache v2 Licensing (chocolateyGH-85) Pass the source to cChocoPackageInstallerSet correctly (chocolateyGH-44) Pass the Source to choco correctly Added version to example (chocolateyGH-77) Push to gallery on new tags (maint) formatting (chocolateyGH-72) PowerShell scripts in format for signing
* development: (build) Updated module version (chocolateyGH-114) Fix typo (chocolateyGH-114) Skip verifying Pester publisher check [chocolatey#105] Chocolatey params passed in when upgrading a package. Choco Run Errors out because of Null Source (chocolateyGH-90) Cache choco list output
|
@mrhockeymonkey You've added two previous commits in your PR. Can you remove those? |
| [OutputType([System.Collections.Hashtable])] | ||
| param ( | ||
| [Parameter(Mandatory)] | ||
| [System.String]$Key, | ||
|
|
||
| [Parameter()] | ||
| [System.String]$Value, | ||
|
|
||
| [Parameter()] | ||
| [ValidateSet('Present', 'Absent')] | ||
| [System.String]$Ensure = 'Present', | ||
|
|
||
| [Parameter()] | ||
| [System.Boolean]$QueryXML |
There was a problem hiding this comment.
The OutputType and Parameter types all use the .NET types. Can we stick with the PowerShell types?
| [System.Boolean]$QueryXML | ||
| ) | ||
|
|
||
| $return = @{ |
There was a problem hiding this comment.
Can you rename this as return is a statement and giving a variable the same name is going to cause confusion.
| [CmdletBinding()] | ||
| [OutputType([System.Collections.Hashtable])] | ||
| param ( | ||
| [Parameter(Mandatory)] |
There was a problem hiding this comment.
To avoid confusion can we use Mandatory = $true?
| if ($Ensure -eq 'Absent') { | ||
| # if value and absent specified remove value as it can be confusing output | ||
| $return['Value'] = $null | ||
| } |
There was a problem hiding this comment.
I'm confused a little but this. The comment says 'if value and absent specified ...' but the if statement only checks for absent. As $Value is an optional parameter it might not be provided.
| function Set-TargetResource { | ||
| [CmdletBinding()] | ||
| param ( | ||
| [Parameter(Mandatory)] |
| ) | ||
|
|
||
| if ($QueryXML.IsPresent) { | ||
| Write-Verbose "Querying chco config via chocolatey.config xml..." |
| $ConfigValue = $ConfigXml.SelectSingleNode($XPath).value | ||
| } | ||
| catch { | ||
| $PSCmdlet.ThrowTerminatingError($_) |
There was a problem hiding this comment.
Just use a throw here rather than the .NET
| try { | ||
| $ConfigXml = [xml](Get-Content -Path $ConfigXmlPath) | ||
| # xpath is case sensitive so we must convert all to lower case | ||
| $XPath = "/chocolatey/config/add[translate(@key, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz') = '$($Key.ToLower())']" | ||
| $ConfigValue = $ConfigXml.SelectSingleNode($XPath).value | ||
| } | ||
| catch { | ||
| $PSCmdlet.ThrowTerminatingError($_) | ||
| } |
There was a problem hiding this comment.
I'm a little unsure about this. The code catches any exceptions being thrown but just throws the same exception. This would be no different than not having the try / catch block. So I'm wondering why it's there?
| } | ||
| else { | ||
| Write-Verbose "Querying choco config via CLI" | ||
| $ConfigValue = & choco config get $Key -r |
There was a problem hiding this comment.
Please don't use aliases (&)? Can you also use the full named Chocolatey switches - so instead of -r use --limit-output.
There is no sanity check being done on the output of this command. What if the key does not exist? The caller will get a response back that is not what they are expecting.
| # limitations under the License. | ||
|
|
||
| #----------------------------------------# | ||
| # Pester tests for cChocoPackageInstall # |
|
This is being superceded by #145. Closing. |
This is in answer to #20 and partly related to #124.
This resource allows control over config settings via "choco config set/unset. It also has an option for querying the xml settings file directly. This is for performance reasons as discussed in #124. It is optional and not recommended for ongoing management but makes the resource much quicker, 2.82 seconds vs 0.68 seconds (see sample output below).
Sample output:
feedback welcome.