edx-lint
edx-lint copied to clipboard
Find incorrect context managers
At Python Study Group this week, we looked at context managers. When we got to @contextlib.contextmanager
, we learned that this is wrong:
@contextmanager
def my_context_manager():
#... set up
yield
#... clean up
because the clean up won't execute if an exception happens inside the caller's with-statement.
I grepped edx-platform to see if this happens, and it does! here and here for example.
We also have instances of pointless context managers with no clean up at all (here):
@contextmanager
def lti_consumer_fields_editing_flag(course_id, enabled_for_course=False):
"""
Yields CourseEditLTIFieldsEnabledFlag record for unit tests
Arguments:
course_id (CourseLocator): course locator to control this feature for.
enabled_for_course (bool): whether feature is enabled for 'course_id'
"""
RequestCache.clear_all_namespaces()
CourseEditLTIFieldsEnabledFlag.objects.create(course_id=course_id, enabled=enabled_for_course)
yield
It would be cool to lint for these mistakes.
BTW: A reason to have a cleanup-less context manager is in a method overriding a correct context manager, but the subclass needs no clean up. If we write this linter, that case can use a disabling pragma.
/cc @cpennington @jmbowman @BessieSteinberg