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

Missing instruction for integrating into PostgREST #19

Open
steve-chavez opened this issue Oct 21, 2023 · 9 comments
Open

Missing instruction for integrating into PostgREST #19

steve-chavez opened this issue Oct 21, 2023 · 9 comments

Comments

@steve-chavez
Copy link
Member

Problem

Related to PostgREST/postgrest#1698 (comment).

Solution

Instructions need to be added to the README.

I assume we need a wrapper function that calls get_postgrest_openapi_spec? Then define it in https://postgrest.org/en/stable/references/configuration.html#db-root-spec as:

db-root-spec = "wrapper_func"
@wayland
Copy link
Contributor

wayland commented Oct 27, 2023

I had a bit of a go at this, and came up with

create or replace function callable_root() returns jsonb as $$
        SELECT get_postgrest_openapi_spec(
                schemas := string_to_array(current_setting('pgrst.db_schemas', TRUE), ','), 
                version := 'not-spec'
        );
$$ language sql;

The problem seems to be that pgrst.db_schemas is coming back as null. If I replace the call to current_setting with the name of the schema I'm using, then it works.

Can we get PostgREST to do a set_config before running this?

@steve-chavez
Copy link
Member Author

Can we get PostgREST to do a set_config before running this?

Yeah, to make this work I think we need to do set_config for various static configs. We also need this for the version as discusssed on #14 (comment)

@laurenceisla WDYT?

@laurenceisla
Copy link
Member

laurenceisla commented Oct 27, 2023

Great! That would solve many problems. Then, the go to would be to query configs from the DB by setting those values every request done to the root/openapi endpoint in PostgREST.

@steve-chavez
Copy link
Member Author

Then, the go to would be to query configs from the DB by setting those values every request done to the root/openapi endpoint in PostgREST.

No, not really. I was thinking we need to initialize the pool connections with those settings. IIRC we did this before for app.settings.* but then we didn't consider pool restarting (the values got lost then).

@wayland
Copy link
Contributor

wayland commented Oct 27, 2023

In a conversation at PostgREST/postgrest#1698 (comment) , I'd asked:

Is there a way I can configure PostgREST to call one of my functions when the config is updated?

...and @steve-chavez replied:

Yes, check https://postgrest.org/en/stable/references/api/openapi.html#override-openapi

...but that's not actually the answer I was looking for -- I wanted to run a query when one of the events in https://postgrest.org/en/stable/references/configuration.html#configuration-reloading happens. Is that at all achievable?

Thanks!

@laurenceisla
Copy link
Member

IIRC we did this before for app.settings.* but then we didn't consider pool restarting (the values got lost then).

Ah, but wouldn't the pool restarting still be a problem? The request is done by PostgREST after all, or am I missing something?

@wayland
Copy link
Contributor

wayland commented Oct 27, 2023

Here's a list of the ones I think we'll need:

  • pgrst.server_host
  • pgrst.server_port
  • pgrst.db_schemas

And renamed from pgrst:

  • openapi.mode
  • openapi.security-active

...and two new ones I think we should have (which would go straight into the info block):

  • openapi.api-name
  • openapi.api-description

...and maybe even:

  • openapi.version (also for the info block)

Ideally, there'd be a system where there'd be a bunch of config files, including pgrst.conf and openapi.conf, and the setting names would be derived from filename + settingname. I'm happy if we don't end up with that setup, but a man can dream :p

Edit: Having looked at the other thread, we could use server_proxy_uri instead if that's a preferred option (I'll have to tear out some of my code, but that's OK :-) ).

HTH,

@wayland
Copy link
Contributor

wayland commented Oct 28, 2023

OK, I've had another bit of a look around. How would this be for an idea:

  • We add to the end of postgres.conf the line include_dir 'conf.d' (or we could just include /usr/local/share/postgres/extensions, and then put .conf files in there), as per https://www.postgresql.org/docs/current/config-setting.html
  • In whichever directory we end up including, we put the openapi configuration parameters I've mentioned above
  • We only rely on PostgREST for the ones I mentioned above that begin with pgrst

Does that sound like a useful plan?

Edit: Also, I've just created PostgREST/postgrest#3029 asking for the three variables we know we need.

@steve-chavez
Copy link
Member Author

Edit: Also, I've just created PostgREST/postgrest#3029 asking for the three variables we know we need.

@wayland Thanks for consolidating the discussion!

Ah, but wouldn't the pool restarting still be a problem? The request is done by PostgREST after all, or am I missing something?

@laurenceisla Right, I think it might be simpler to just add tx-settings for the special case of the root spec for now. Let's continue on PostgREST/postgrest#3029

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants