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.

Sign up for GitHub

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

Jump to bottom

Make base models abstract #516

Open
SimeonAleksov opened this issue Feb 23, 2022 · 9 comments
Open

Make base models abstract #516

SimeonAleksov opened this issue Feb 23, 2022 · 9 comments

Comments

@SimeonAleksov
Copy link

SimeonAleksov commented Feb 23, 2022 •
edited
Loading

Summary:

The base models should be abstract, allowing us to extend them without creating a messy database.

I would be happy to prepare a pull request if you're willing to make this change.
@auvipy
Copy link
Member

auvipy commented Feb 23, 2022

yes I am positive for that, and as a ref https://github.com/justquick/django-activity-stream/pull/479/files#diff-9bf291553a50e73e92ef5012758947b27e3dae79f7b7ab6a97583b6cf7851c3cR16

@diegocastrum
Copy link
Contributor

Hi @SimeonAleksov! How is it going with this issue? I think I could work on it too if you like the idea or if you're too busy. I need that abstract layer to implement django-celery-beat and control the tasks in a multitenant platform.

@SimeonAleksov
Copy link
Author

Hi @SimeonAleksov! How is it going with this issue? I think I could work on it too if you like the idea or if you're too busy. I need that abstract layer to implement django-celery-beat and control the tasks in a multitenant platform.

Hey, I've already opened PR with basic implementation, #521, if you think there's more feel free to update it as I don't have much time.

@diegocastrum
Copy link
Contributor

Hey, I've already opened PR with basic implementation, #521, if you think there's more feel free to update it as I don't have much time.

Ah nice! I will try to take look of it asap, thanks!

@jose-padin
Copy link

Hello, @auvipy
I found these changes very useful for me too. When do you think the changes can be approved and integrated in the project?
Thanks.

@diegocastrum
Copy link
Contributor

@SimeonAleksov The changes you made are technically enough from my point of view. I've restructured the code related with the models and abstract classes in a way that may be more maintainable following more or less the approach that Django does.

@SimeonAleksov I don't have permissions to update your branch/pull request. So @auvipy and/or @SimeonAleksov what is the best way you suggest to push these changes?

diegocastrum added a commit to diegocastrum/django-celery-beat that referenced this issue Apr 11, 2022
@diegocastrum
Support for abstract base models improved. (celery#516)
49d9331 
Model classes moved into a `models.py` directory, which exports by
default only the default `django-celery-beat` model classes now from
`models.generic.py`.

`managers.py` moved into `models.py` directory, because it's something
that is needed only by models.
@SimeonAleksov
Copy link
Author

@SimeonAleksov The changes you made are technically enough from my point of view. I've restructured the code related with the models and abstract classes in a way that may be more maintainable following more or less the approach that Django does.

@SimeonAleksov I don't have permissions to update your branch/pull request. So @auvipy and/or @SimeonAleksov what is the best way you suggest to push these changes?

I see that you've opened a PR, @auvipy just close mine, his is better.

@diegocastrum
Copy link
Contributor

I see that you've opened a PR, @auvipy just close mine, his is better.

I apologize @SimeonAleksov I was too impatient. I was a bit confused when you said that I may update your pull-request. I learned today that I could "fork your fork" to do that. My bad, I will do it better next time.

diegocastrum added a commit to diegocastrum/django-celery-beat that referenced this issue Jun 3, 2022
@diegocastrum
Bypass de default models used by django_celery_beat.scheduler (cele…
742ee54 
…ry#516)

when
`CELERY_BEAT_(?:PERIODICTASKS?|(?:CRONTAB|INTERVAL|SOLAR|CLOCKED)SCHEDULE)_MODEL`
constants are defined in django `settings`.

Providing the `app_label.model_name` of your own models as value for the
constants
`CELERY_BEAT_(?:PERIODICTASKS?|(?:CRONTAB|INTERVAL|SOLAR|CLOCKED)SCHEDULE)_MODEL`
will let `django_celery_beat.scheduler` use the custom models instead of
the default ones (aka generic models, in this context/pull-request):

```python
CELERY_BEAT_PERIODICTASK_MODEL = "app_label.model_name"
CELERY_BEAT_PERIODICTASKS_MODEL = "app_label.model_name"
CELERY_BEAT_CRONTABSCHEDULE_MODEL = "app_label.model_name"
CELERY_BEAT_INTERVALSCHEDULE_MODEL = "app_label.model_name"
CELERY_BEAT_SOLARSCHEDULE_MODEL = "app_label.model_name"
CELERY_BEAT_CLOCKEDSCHEDULE_MODEL = "app_label.model_name"
```

Doing this we add support to automatically bypass the default
`django_celery_beat` models without forcing developers to overwrite the
whole `django_celery_beat.scheduler` in projects where the default
models doesn't fit the requirements

I updated the `README.rst` with a small explanation about how this work

Additonal information:
* related issue: celery#516
* pull-request: celery#534
@axsaucedo
Copy link

Coming from celery/django-celery-results#314 to add a +1 here as well 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@diegocastrum @axsaucedo @auvipy @SimeonAleksov @jose-padin