Azure / Azure-Data-Factory-Integration-Runtime-in-Windows-Container

Azure Data Factory Integration Runtime in Windows Container Sample
MIT License
25 stars 36 forks source link

Referencing environment variables instead of locally scoped variables #21

Closed pkothare closed 6 months ago

pkothare commented 7 months ago

When running:

> docker run -d -e AUTH_KEY=<ir-authentication-key> \
    [-e NODE_NAME=<ir-node-name>] \
    [-e ENABLE_HA={true|false}] \
    [-e HA_PORT=<port>] \
    [-e ENABLE_AE={true|false}] \
    [-e AE_TIME=<expiration-time-in-seconds>] \
    <image-name>

as defined in the README, the entry point of the container is

ENTRYPOINT ["powershell", "C:/SHIR/setup.ps1"]

This implies ENABLE_HA and HA_PORT are considered environment variables when the main section of setup.ps1 runs. This change references the environment variables $Env:ENABLE_HA and $Env:HA_PORT in the main section instead of setup.ps1 instead of $ENABLE_HA and $HA_PORT since the locally scoped variables are undefined initially.

byran77 commented 6 months ago

wondering why only ENABLE_HA and HA_PORT are included in the changes except other variables

pkothare commented 6 months ago

wondering why only ENABLE_HA and HA_PORT are included in the changes except other variables

@byran77, the problem is scoping. Lines 87-144: https://github.com/Azure/Azure-Data-Factory-Integration-Runtime-in-Windows-Container/blob/b4989a057d180912772b71f2b90c9e463db333db/SHIR/setup.ps1#L87-L144 are not scoped to any function, and thus $ENABLE_HA and $HA_PORT are undefined when the script execution reaches L90 and L92 subsequently. However, $Env:ENABLE_HA and $Env:HA_PORT may be available as a consequence of running the docker run ... command.

This may been an error when it was initially developed, which is why I'm proposing the changes. It also potentially resolves #3.

byran77 commented 6 months ago

wondering why only ENABLE_HA and HA_PORT are included in the changes except other variables

@byran77, the problem is scoping. Lines 87-144:

https://github.com/Azure/Azure-Data-Factory-Integration-Runtime-in-Windows-Container/blob/b4989a057d180912772b71f2b90c9e463db333db/SHIR/setup.ps1#L87-L144

are not scoped to any function, and thus $ENABLE_HA and $HA_PORT are undefined when the script execution reaches L90 and L92 subsequently. However, $Env:ENABLE_HA and $Env:HA_PORT may be available as a consequence of running the docker run ... command. This may been an error when it was initially developed, which is why I'm proposing the changes. It also potentially resolves #3.

make sense, thanks @pkothare