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.

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

Unable to override the default base Task class for tasks registered without the base option #76

Closed
sanchit-mathew opened this issue Apr 23, 2023 · 3 comments · Fixed by #78

Comments

@sanchit-mathew
Copy link

sanchit-mathew commented Apr 23, 2023

Context:

  1. We have overridden the TenantTask class provided by tenant-schemas-celery for our use-case - consider it to be named as DerivedTenantTask
  2. There exists a task, defined as, for example:
@app.task
def my_task(arg1, arg2):
    ...

Issue:

  • tenant-schemas-celery's CeleryApp.create_task_cls hardcodes "tenant_schemas_celery.task:TenantTask" as the base class that all tasks will be subclassed from
  • Due to this, even though we're specifying our custom DerivedTenantTask as the base task_cls for our celery app, all tasks where the base is not explicity mentioned in the @app.task decorator are being subclassed from tenant-schemas-celery's TenantTask and due to this, the custom logic we've written in our DerivedTenantTask class is not being executed

Request:

  • Please keep the celery app's task_cls attribute in tenant-schemas-celery's CeleryApp.create_task_cls:
class CeleryApp:
    registry_cls = 'tenant_schemas_celery.registry:TenantTaskRegistry'
    task_cls = "tenant_schemas_celery.task:TenantTask`

    def create_task_cls(self):
        return self.subclass_with_self(
            self.task_cls,
            abstract=True,
            name="TenantTask",
            attribute="_app",
        )
  • LMK if this was kept intentionally or if you agree that this can be fixed
  • If it can be fixed, also LMK if you're running short on bandwidth and would like me to pick this up myself
@maciej-gol
Copy link
Owner

Thanks for reporting! This indeed looks like an omission. Will look into it!

@sanchit-mathew
Copy link
Author

@maciej-gol Any updates re. this?

@maciej-gol
Copy link
Owner

@sanchit-mathew can you please verify if #78 this is would work for you?

maciej-gol added a commit that referenced this issue May 21, 2023
* CeleryApp allows overriding the task class.

Fixes #76

* Don't set test app as current
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants