Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Env vars substitution problem #9

Closed
andgineer opened this issue Jun 12, 2023 · 3 comments
Closed

Env vars substitution problem #9

andgineer opened this issue Jun 12, 2023 · 3 comments
Assignees

Comments

@andgineer
Copy link

In ENTRYPOINT we load config files and substitute env vars.
Unfortunately we first load the configs as JSON and only after that we substitute ${}

That means we cannot use env vars for non-string values

For example "IndexConnectionsCount" : ${ORTHANC_DB_INDEX_CONNECTIONS_COUNT}
because this is not valid JSON.
And if we enclose the value to quotes that will be an invalid int value in the config.

Maybe we should consider using a more straightforward substitution approach like just envsubst for the files

@amazy
Copy link
Member

amazy commented Jun 14, 2023

Seems like we could indeed call envsubst from python before parsing the file

for filePath in configFiles:
  logInfo("reading configuration from " + filePath)
  with open(filePath, "r") as f:
    content = f.read()

------------ here -------------

    try:
      cleanedContent = removeCppCommentsFromJson(content)
      configFromFile = json.loads(cleanedContent)
    except:
      logError("Unable to parse Json file '{f}'; check syntax".format(f = filePath))
    
    configurator.mergeConfigFromFile(configFromFile, filePath)

The substitution that happens afterwards is somehow different since it uses Env var names to add values at the right place in the config file.

@andgineer
Copy link
Author

that would be nice thing to have
or may be to keep backward compatibility we can do that only on json load exception

@amazy
Copy link
Member

amazy commented Jun 14, 2023

it is safe to do it in anycase.
Fixed in 6e6c105

@amazy amazy closed this as completed Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants