Repository navigation
Conversation
Sets up the Postgres side of the subscription store: settings for the app and migrations URLs, a pgbouncer-safe engine, the subscriptions table, and a migrate command. The devserver and CI get a local Postgres.
pbhandari
left a comment
pbhandari
left a comment
There was a problem hiding this comment.
I have some pretty strong objections to us introducing a completely separate method of doing these migrations. This is a pretty big footgun we'd always have to remember whenever we're working with subscriptions. I'd very much prefer if this was done using the existing migraition system.
If the addition of postgres, and it's migrations, make our tooling assumptions invalid, then the tooling assumptions to be updated.
At the very least I'd like to make sure that snuba migrate continues to behave as expected. Happy to get on a call to discuss options to get us to that state.
| SUBSCRIPTIONS_POSTGRES_URL = os.environ.get("SUBSCRIPTIONS_POSTGRES_URL") | ||
| SUBSCRIPTIONS_POSTGRES_MIGRATIONS_URL = os.environ.get("SUBSCRIPTIONS_POSTGRES_MIGRATIONS_URL") |
There was a problem hiding this comment.
While we are currently using postgres for subscriptions; we shouldn't bake that into the name. I'd much prefer if we dropped teh subscriptions prefix.
resolves INFRENG-678
Adds a Postgres connection and alembic migrations for the subscription store, with a first migration creating the
subscriptionstable and asnuba subscriptions-postgres migratecommand to run it. Nothing reads or writes the table yet; that comes with the store itself in INFRENG-679.I put the migrations in
snuba/subscriptions/postgres/migrations/, next to the code that'll use them, and they run on plain alembic. They have nothing to do with snuba's ClickHouse migration system: there's no group to register, andsnuba migrationsnever sees them. I did consider putting them undersnuba/snuba_migrations/so all of snuba's migrations live in one place, but the tooling there assumes ClickHouse (check-migrations.pywould fail this PR for mixing migrations with other code, andddl-changes.pywould parse them as ClickHouse DDL). Keeping them with the feature is also how sentry does it with Django, where each app has its ownmigrations/folder. If you'd rather have everything in one place, I'm happy to move them.