search

supabase / postgres

Unmodified Postgres with some useful plugins
https://supabase.com
PostgreSQL License
1.35k stars 136 forks source link

feat: enable webhooks by default #1152

Open sweatybridge opened 3 weeks ago

sweatybridge commented 3 weeks ago

SQL migrated over from cli / api https://github.com/supabase/cli/blob/develop/internal/db/start/templates/schema.sql#L26 ~With improved retry support from @mansueli: https://github.com/supabase/infrastructure/pull/16831~

Solves an immediate pain point with enabling webhooks. More context: https://supabase.slack.com/archives/C07E5GFAHTM/p1724229124279079

Upgrade path for existing projects can be done via ansible rollout as the migration is idempotent.

soedirgo commented 3 weeks ago

Please omit the retry changes for now, the Postgres team doesn't have the bandwidth to fix it if it goes south.

@steve-chavez would it be an issue to enable Database Webhooks by default once we package it as an extension?

steve-chavez commented 3 weeks ago

@steve-chavez would it be an issue to enable Database Webhooks by default once we package it as an extension?

I'm a bit confused about this change. So this means that the supabase_functions schema will be created by default and not with a manual step on the dashboard?

If so, is certainly an improvement, but ideally we package supabase_functions as an extension and not a "migration". This is so we can control the SQL for debugging and upgrading. So far this has been impossible to do for me.

So could we move it to nix/ext/supabase_functions, make it a extension (v0.1) and make it depend on pg_net?

This would be a huge improvement.

steve-chavez commented 3 weeks ago

So could we move it to nix/ext/supabase_functions, make it a extension (v0.1) and make it depend on pg_net?

If the above can be done without a breaking change later, I can help with it after I finish https://github.com/supabase/pg_net/pull/139.

sweatybridge commented 3 weeks ago

Yes, this removes the manual step from enabling on dashboard.

Enabling webhooks is also currently irreversible since we don't support disabling from dashboard. That's why I think it's ok to have it as a migration since we have to deal with the upgrade path for existing projects anyway.

But I agree a separate extension is better. I believe the supabase_functions schema is currently locked down to supabase_admin so it's safe to switch without breaking.

This webhook also creates the supabase_functions_admin role. Do we want to create it in the extension too?

mansueli commented 3 weeks ago

+1 for moving into an extension

Having it as an extension will also make it easier to release updates later on (like the retry feature)

Sent from Outlook for Androidhttps://aka.ms/AAb9ysg

From: Han Qiao @.> Sent: Friday, August 23, 2024 2:52:07 AM To: supabase/postgres @.> Cc: Rodrigo Mansueli @.>; Mention @.> Subject: Re: [supabase/postgres] feat: enable webhooks by default (PR #1152)

Yes, this removes the manual step from enabling on dashboard.

Enabling webhooks is also currently irreversible since we don't support disabling from dashboard. That's why I think it's ok to have it as a migration since we have to deal with the upgrade path for existing projects anyway.

But I agree a separate extension is better. I believe the supabase_functions schema is currently locked down to supabase_admin so it's safe to switch without breaking.

This webhook also creates the supabase_functions_admin role. Do we want to create it in the extension too?

— Reply to this email directly, view it on GitHubhttps://github.com/supabase/postgres/pull/1152#issuecomment-2306342162, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABGNTEHZKARL7HJFTPEIEUTZS3EYPAVCNFSM6AAAAABM4SH3TGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBWGM2DEMJWGI. You are receiving this because you were mentioned.Message ID: @.***>

steve-chavez commented 3 weeks ago

I believe the supabase_functions schema is currently locked down to supabase_admin so it's safe to switch without breaking.

Ok cool. Then this should be good to go.

This webhook also creates the supabase_functions_admin role. Do we want to create it in the extension too?

Haven't thought much about this, maybe the role creation and the privileges should remain as migration. I don't recall seeing a pg extension that creates roles (since they're global objects). But is something for later.

soedirgo commented 3 weeks ago

pg extension that creates roles

pgsodium does this, but yeah agree that creating global objects in an extension is a bit awkward

steve-chavez commented 1 week ago

Isn't pg_net still seeing segfaults? Do we really want to enable it by default?

No more segfaults since https://github.com/supabase/pg_net/releases/tag/v0.9.3, those were fixed on https://github.com/supabase/pg_net/pull/136.