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

Can't set app settings when using environment variables #1624

Closed
LorenzHenk opened this issue Oct 15, 2020 · 12 comments · Fixed by #1691
Closed

Can't set app settings when using environment variables #1624

LorenzHenk opened this issue Oct 15, 2020 · 12 comments · Fixed by #1691
Assignees
Labels
bug config related to the configuration options

Comments

@LorenzHenk
Copy link

Environment

  • PostgreSQL version: 13
  • PostgREST version: v7.0.1

Description of issue

The documentation mentions the app.settings.* settings, where one can set custom session variables.

How can this setting be set when configuring PostgREST through environment variables?

I tried:

  • PGRST_APP.SETTINGS.TEST - this is not allowed in the shell
  • PGRST_APP_SETTINGS_TEST - this is not recognized by PostgREST
@steve-chavez
Copy link
Member

@LorenzHenk Are you on docker?

Otherwise the config should accept env vars like:

app.settings.test = "$(A_TEST)"

(From: http://postgrest.org/en/v7.0.0/configuration.html#app-settings)

@LorenzHenk
Copy link
Author

Yes I'm using docker & docker-compose.

I'm not using a config file. Instead, I'm using environment variables (like PGRST_DB_URI) for configuration.

@steve-chavez
Copy link
Member

steve-chavez commented Oct 17, 2020

I see. Inside Docker, right now the config file is the only way to use the app.settings.* configs.

You could do it like:

docker run -v /absolute/path/to/config:/etc/postgrest.conf postgrest/postgrest

This is a limitation because we currently use a static postgrest.conf inside Docker. We interpolate the env vars inside that config.

@steve-chavez
Copy link
Member

One way we could solve this is by changing the app.settings config to use a single value(thinking json for now) for all the settings.

In the regular binary, one would use like:

app-settings = "{'one': '$(ONE)', 'two': '$(TWO)'}"

On Docker:

PGRST_APP_SETTINGS = "{'one': 'value', 'two': 'other value'}"

Would that work for you?

@steve-chavez
Copy link
Member

The other way to solve this would be to make our binary also understand command line arguments instead of only the config file. But I think that would be more work since we'd need to integrate an additional library.

@wolfgangwalther
Copy link
Member

I think we should just separate the config file from the environment variables. If we read those in a second step manually, we can easily read all of the custom settings with a pattern like PGRST_APP_SETTINGS_xxx.

Environment variables should override those values, that were set in the config file. Including environment variables with the current syntax in the config file would still be possible, because that's part of the lib we're using, right?

Since we would be parsing the environment variables ourself, that would solve #1572 as well.

@LorenzHenk
Copy link
Author

@steve-chavez I think changing the app.settings has several drawbacks:

  • breaking change for everybody
  • worse readability of config file
  • ambiguous nested values, e.g.:
    {'one': {'nested': 'value'}}
    
    vs
    {'one.nested': 'value'}
    

@wolfgangwalther I think having PGRST_APP_SETTINGS_* would be the way to go.

@steve-chavez
Copy link
Member

If we read those in a second step manually, we can easily read all of the custom settings with a pattern like PGRST_APP_SETTINGS_xxx

@wolfgangwalther So we create the config file dynamically on Docker right? That certainly looks like a better solution.

@wolfgangwalther
Copy link
Member

@wolfgangwalther So we create the config file dynamically on Docker right? That certainly looks like a better solution.

Yes, the docker container would not need a config file at all anymore. All env variables directly read into AppConfig.

@wolfgangwalther wolfgangwalther added the config related to the configuration options label Nov 22, 2020
@wolfgangwalther wolfgangwalther self-assigned this Dec 1, 2020
wolfgangwalther added a commit to wolfgangwalther/postgrest that referenced this issue Dec 13, 2020
wolfgangwalther added a commit to wolfgangwalther/postgrest that referenced this issue Dec 13, 2020
wolfgangwalther added a commit to wolfgangwalther/postgrest that referenced this issue Dec 13, 2020
wolfgangwalther added a commit to wolfgangwalther/postgrest that referenced this issue Dec 13, 2020
wolfgangwalther added a commit to wolfgangwalther/postgrest that referenced this issue Dec 13, 2020
wolfgangwalther added a commit to wolfgangwalther/postgrest that referenced this issue Dec 13, 2020
wolfgangwalther added a commit to wolfgangwalther/postgrest that referenced this issue Dec 23, 2020
monacoremo pushed a commit to monacoremo/postgrest that referenced this issue Jul 17, 2021
@tcarr-harriscomputer
Copy link

@wolfgangwalther , this change broke my docker-setup where I was using the documented https://postgrest.org/en/stable/configuration.html?highlight=app.settings#app-settings app.settings.xxx in my /etc/postgrest.conf via a docker-compose volume override.

I think your new environment variables are more elegant and easier, but would it be possible to update the documentation about PGRST_APP_SETTINGS_xxx ?

@wolfgangwalther
Copy link
Member

The docs you are referencing are for v7. We have not released the docs for v8, yet. @laurenceisla is working hard on those docs right now, I'm sure this will be included soon.

@laurenceisla
Copy link
Member

@tcarr-harriscomputer As Wolfgang mentioned, the docs for v8 are now available. You can find a section for Environment Variables and an updated section for Docker configuration as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug config related to the configuration options
Development

Successfully merging a pull request may close this issue.

5 participants