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

@set_code_owner_attribute clashes with django-user-tasks #349

Open
kdmccormick opened this issue Oct 5, 2023 · 4 comments
Open

@set_code_owner_attribute clashes with django-user-tasks #349

kdmccormick opened this issue Oct 5, 2023 · 4 comments

Comments

@kdmccormick
Copy link
Member

(I'm not sure whether this is an issue with edx-django-utils, with django-user-tasks, or both...)

Adding @set_code_owner_attribute and @shared_task to the same function causes errors when that function is an instance of UserTask.

For this reason, the import_olx and export_olx tasks in edx-platform do not use @set_code_owner_attribute: https://github.com/openedx/edx-platform/blob/db252978f3f315c896b07443f70febdc043faee4/cms/djangoapps/contentstore/tasks.py#L310-L311

I recently (accidentally) confirmed this issue... I was working on update_children_task in the content_libraries djangoapp, and had this:

@shared_task(base=LibraryUpdateChildrenTask, bind=True)
@set_code_owner_attribute
def update_children_task(self, user_id, dest_block_key, version=None):
    ....

The celery task did still work, but logs contained a background exception:

 Traceback (most recent call last):
   File "/openedx/venv/lib/python3.8/site-packages/celery/utils/dispatch/signal.py", line 276, in send
     response = receiver(signal=self, sender=sender, **named)
   File "/openedx/venv/lib/python3.8/site-packages/user_tasks/signals.py", line 215, in start_user_task
     sender.status.start()
   File "/openedx/venv/lib/python3.8/site-packages/user_tasks/tasks.py", line 84, in status
     name = self.generate_name(arguments_dict)
   File "/openedx/edx-platform/openedx/core/djangoapps/content_libraries/tasks.py", line 274, in generate_name
     key = arguments_dict['dest_block_key']
 KeyError: 'dest_block_key'

Digging in, I found that this key is missing because the django-user-tasks-defined function arguments_as_dict was not properly returning a dictionary. The underlying failure seems to be that in the call inspect.getcallargs(cls.run, ...), cls.run does not always actually equal the function defining the celery task (update_children_task), but rather the stub method Task.run. I imagine that this has to do with @set_code_owner_attribute iterfering with the introspection that @shared_task and/or django-user-tasks relies upon.

I am not exactly sure what the react or impact of this bug is. I do know for sure that developers should not use @set_code_owner_attribute on any Django user task function.

@kdmccormick kdmccormick changed the title @set_code_owner_attribute clashes with user tasks @set_code_owner_attribute clashes with django-user-tasks Oct 5, 2023
@robrap
Copy link
Contributor

robrap commented Oct 5, 2023

This has been a problem for certain tasks. See https://edx.readthedocs.io/projects/edx-django-utils/en/latest/monitoring/how_tos/add_code_owner_custom_attribute_to_an_ida.html#handling-celery-tasks for the alternative that avoids the decorator.

@robrap
Copy link
Contributor

robrap commented Oct 5, 2023

@timmc-edx: I don't know if this affects your linting.

@timmc-edx
Copy link
Contributor

It doesn't really affect the linting in edx-platform, no -- it will accept either approach.

@kdmccormick
Copy link
Member Author

Got it, thanks for the alternative. I see now that import_olx and export_olx both use that alternative.

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

No branches or pull requests

3 participants