fix: turn off atomic requests for course api get views #33230
Conversation
ormsbee
left a comment
ormsbee
left a comment
There was a problem hiding this comment.
Decorator application looks fine. I would like to better understand why the transaction.atomic() was needed in the test case though.
| # Django TestCase wraps every test in a transaction, so we must specifically wrap this when we expect an error | ||
| with transaction.atomic(): | ||
| response = self.client.get(url) |
There was a problem hiding this comment.
Interesting. I wouldn't have expected this to cause a problem. What exception gets thrown without this?
There was a problem hiding this comment.
There was a problem hiding this comment.
==================================== ERRORS ====================================
_____ ERROR at teardown of CourseHomeMetadataTests.test_get_unknown_course _____
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/waffle/testutils.py:41: in disable
self.obj.delete()
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/waffle/models.py:117: in delete
ret = super().delete(*args, **kwargs)
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/base.py:[96](https://github.com/openedx/edx-platform/actions/runs/6162508221/job/16724140608#step:7:100)7: in delete
return collector.delete()
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/deletion.py:410: in delete
count = qs._raw_delete(using=self.using)
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/query.py:762: in _raw_delete
cursor = query.get_compiler(using).execute_sql(CURSOR)
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/sql/compiler.py:1175: in execute_sql
cursor.execute(sql, params)
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py:66: in execute
return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py:75: in _execute_with_wrappers
return executor(sql, params, many, context)
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py:78: in _execute
self.db.validate_no_broken_transaction()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <django.db.backends.sqlite3.base.DatabaseWrapper object at 0x7f149c480430>
def validate_no_broken_transaction(self):
if self.needs_rollback:
> raise TransactionManagementError(
"An error occurred in the current transaction. You can't "
"execute queries until the end of the 'atomic' block.")
E django.db.transaction.TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/base/base.py:447: TransactionManagementError
There was a problem hiding this comment.
Okay. Pure paranoia, but on your local machine when you're using runserver and try to access a non-existent course, it gives you the expected 404, right?
ormsbee
left a comment
ormsbee
left a comment
There was a problem hiding this comment.
I have one paranoia question around the error thrown by runserver when trying to fetch a missing course. As long as that's sane, 
|
|
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
* fix: turn off atomic requests for course api get views * test: fix test
https://2u-internal.atlassian.net/browse/AU-1450
Deadlock errors for
lms.djangoapps.course_home_api.course_metadata.views:CourseHomeMetadataView.getandopenedx.core.djangoapps.courseware_api.views:CoursewareInformation.getare the number 2 and 5 most common error we see.The views do no writing besides some minor user tracking, so we can simply mark them as nonatomic