fix: use default for DATAHUB_GMS_PROTOCOL #61
Conversation
| @@ -44,4 +44,4 @@ USER datahub | |||
| RUN curl -s "https://get.sdkman.io" | bash | |||
| RUN /bin/bash -c "source /$HOME/.sdkman/bin/sdkman-init.sh; sdk version; sdk install java 8.0.332-zulu" | |||
| ENV DATAHUB_GMS_PROTOCOL http | |||
| CMD dockerize -wait $DATAHUB_GMS_PROTOCOL://$DATAHUB_GMS_HOST:$DATAHUB_GMS_PORT/health -timeout 240s /start_datahub_actions.sh | |||
| CMD dockerize -wait ${DATAHUB_GMS_PROTOCOL:-http}://$DATAHUB_GMS_HOST:$DATAHUB_GMS_PORT/health -timeout 240s /start_datahub_actions.sh | |||
There was a problem hiding this comment.
was tempting to do this, but then we lose the option to CMD ["..", ] for proper signal handling.
There was a problem hiding this comment.
the current line works because it uses "sh -c" in the background that resolves the default env var. if we want to use CMD [".."] in the future then we cannot do this afaik. (I couldn't make it work dockerize, but that's an independent problem)
There was a problem hiding this comment.
As per https://stackoverflow.com/questions/40454470/how-can-i-use-a-variable-inside-a-dockerfile-cmd, CMD [".."] doesn't do any env expansion
Given that we have DATAHUB_GMS_HOST and DATAHUB_GMS_PORT in the command, we already require a shell, and hence it's probably ok to rely on the env var expansion of sh -c for DATAHUB_GMS_PROTOCOL too
There was a problem hiding this comment.
tried to make CMD [".."], so the container can handle signals (e.g ctrl-C), but dockerize doesn't work for some reason that way. This shouldn't block you merging this.
Co-authored-by: Peter Szalai <szalaipeti.vagyok@gmail.com>
No description provided.