microsoft / nav-docker

Official Microsoft repository for Dynamics NAV in Docker resources. It has not been decided yet, to which extend Microsoft will ship Docker images with NAV, so everything in this repo is work in progress and might be subject to deletion.
MIT License
179 stars 92 forks source link

Helper Function to set settings - potential better way to extract keyvalue pair? #562

Open KristofKlein opened 1 month ago

KristofKlein commented 1 month ago

Hi,

I was playing around with docker compose to start Business Central container, and wanted to add the InstrumentationString.

This failed as the Instrumentation string itself consists out of several key/value pairs split by a '=' ... the function to set customSettings can not handle this.

so I was wondering if it would make sense to replace that logic with something like :


function Set-ConfigSetting { ...}
  foreach ($customSetting in $customSettingsArray) {
        # not so good:
        # $customSettingArray = $customSetting -split "="
        # $customSettingKey = $customSettingArray[0]
        # $customSettingValue = $customSettingArray[1]

        # maybe better ?!: mind the double assignment and the ,2
        $customSettingKey , $customSettingValue = $customSetting -split "=",2 

OutPut old code:

ApplicationInsightsConnectionString
InstrumentationKey (<- this is what it actually set on the ST Server)
GUUUUUIDDDD;IngestionEndpoint
https://SOME.applicationinsights.azure.com;LiveEndpoint
https://SOME.monitor.azure.com;ApplicationId
MOREGUUUID

Outpu new code:

ApplicationInsightsConnectionString
InstrumentationKey=GUUUUUUID;IngestionEndpoint=https://SOME.applicationinsights.azure.com;LiveEndpoint=https://SOME.monitor.azure.com;ApplicationId=MOREGUUID

Which looks more correct

freddydk commented 1 week ago

You are right, that if the key or the value contains a = - then the logic doesn't work and since the key never contains a =, your suggestion should be fine. Feel free to create a PR with this. I will deploy a preview version and it will be included in the next update (around mid september)

KristofKlein commented 1 week ago

I really hope I did it right :D