PostgREST / postgrest-openapi

OpenAPI output generated in SQL for a PostgREST instance
MIT License
17 stars 4 forks source link

Missing instruction for integrating into PostgREST #19

Open steve-chavez opened 11 months ago

steve-chavez commented 11 months ago

Problem

Related to https://github.com/PostgREST/postgrest/issues/1698#issuecomment-1773676752.

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 commented 11 months ago

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 commented 11 months ago

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 https://github.com/PostgREST/postgrest-openapi/issues/14#issuecomment-1773934917

@laurenceisla WDYT?

laurenceisla commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

In a conversation at https://github.com/PostgREST/postgrest/issues/1698#issuecomment-1773676752 , 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 commented 11 months ago

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 commented 11 months ago

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

And renamed from pgrst:

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

...and maybe even:

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 commented 11 months ago

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

Does that sound like a useful plan?

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

steve-chavez commented 11 months ago

Edit: Also, I've just created https://github.com/PostgREST/postgrest/issues/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 https://github.com/PostgREST/postgrest/issues/3029