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

make bookmarks update_xblocks_cache task name match function #508

Conversation

thraxil
Copy link

@thraxil thraxil commented Dec 27, 2019

Investigating some of the celery related sentry errors like this one: https://sentry.io/organizations/appsembler/issues/1024724752/events/61b07188c79d4a579fafc712953e3a85/?project=87786

I still don't know exactly what's causing them, but I did notice that in this case, the name assigned to the task ....update_xblock_cache didn't match the actual function name, update_xblocks_cache(). Everywhere that it's called elsewhere in the codebase, eg here: https://github.com/appsembler/edx-platform/blob/appsembler/tahoe/master/openedx/core/djangoapps/bookmarks/signals.py#L20 it is referenced as update_xblocks_cache.

So, I don't know if this will actually fix that error (it probably won't; there are a bunch of similar exceptions that lead me to believe it's something more related to how the celery workers are configured), but it certainly seems like it was a typo and wouldn't hurt to make it more consistent.

Copy link

@OmarIthawi OmarIthawi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for jumping on this. I gotta admit we stalled it for a pretty long time.

While this might fix the error it probably won't survive the next release upgrade. Please see #510.

@OmarIthawi
Copy link

Closing in favor of #510. Thank you again Anders!

@OmarIthawi OmarIthawi closed this Dec 30, 2019
@OmarIthawi OmarIthawi deleted the bookmarks-update-xblock-cache-task-name branch December 30, 2019 10:00
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 this pull request may close these issues.

2 participants