farahats9 / sqlalchemy-celery-beat

Celery Periodic Tasks backed by the SQLAlchemy
MIT License
38 stars 6 forks source link

Question: Is it possible to not create celery tables on start? #14

Open miguelvalente opened 4 weeks ago

miguelvalente commented 4 weeks ago

My issue is that when deploying a project I first need to up my app and app_db container and migrate, with celery tables. And then only up the workers after I migrate.

Maybe there's a better way of doing the above anyway. Any help/clarification on this is appreciated as always.

farahats9 commented 4 weeks ago

I am not sure I understand the issue, the required tables get created on starting celery beat process, does this not happen for you or do you want it to happen before the process start?

I have been planning to add migrations using Alembic to address this concern but don't have the time, any PR is appreciated though.

miguelvalente commented 4 weeks ago

I would like to avoid the creation of tables when the beats start. So as to allow alembic to take care of it.

Problem:


I have made a celery_tables.py and pointed alembic to it. Is this the kinda stuff you need? Or you mean the migration script? celery_tables.py.txt

farahats9 commented 4 weeks ago

I haven't decided how it should be migrated yet, I feel for most use cases having the tables created on start make sense. But if we add alembic there will be additional step required.

Regarding your question how it should look, I think just like any alembic migrations, it can be created automatically with some alembic commands but I am not sure how to distribute it to end users, a quick search on stackoverflow shows there is a way to do that

I will look into this more when I have some time, feel free to try it, and if it works for you try to push your findings into a PR.

de-shev commented 1 week ago

There is an example of integration between GeoAlchemy2 and Alembic which you could consider.

farahats9 commented 1 week ago

I looked at the example you provided, it is the same way like Alembic official documentation, which is the way I already thought of doing it. The problem is that it will be necessary to have additional steps before the initial run, and I feel that may complicate things for an average user.

My thoughts are to have 2 ways, the current way and an optional cli command to generate offline migration, that way it can solve this issue,

I will push my tested code very soon since I already got it working-ish.

de-shev commented 1 week ago

Can you share your demo solution or hacks to make this work? I tried adding 'metadata' to 'target_metadata', but Alembic creates tables using the "celery_schema" schema and generates incorrect migrations. It does this without using "CREATE SCHEMA IF NOT EXISTS celery_schema". If I add this line manually and run the migration, everything works fine, but when creating a new migration, it doesn't detect changes in the database and creates a duplicate migration. Additionally, when rolling back, I encounter the error: type "period" already exists.

from infrastructure.db.database import Base
from sqlalchemy_celery_beat.session import ModelBase as CeleryBeatModelBase
target_metadata = [Base.metadata, CeleryBeatModelBase.metadata]
miguelvalente commented 1 week ago

I have two instances of metadata. You can 100% make this better.

from src.database import metadata as metadata_database
from src.celery_core.celery_tables import metadata as metadata_celery_tables

config = context.config

if config.config_file_name is not None:
    fileConfig(config.config_file_name)

combined_metadata = MetaData()
for metadata in [metadata_database, metadata_celery_tables]:
    for table in metadata.tables.values():
        combined_metadata._add_table(table.name, table.schema, table)

target_metadata = combined_metadata

Regarding the: type "period" already exists.

You just have to make sure your migrations dont try to create enums. And if downing a migration version to make sure that the enums are being deleted.

But honestly I'm getting pretty fing tired of alembic and auto generated migrations because if it goes beyond adding tables theres a chance that you have to go migration file itself and fix shit.