docs: update sys.path hacks ADR & add upgrade guide

The timeline of the import_shims removal has evolved
over time; notably, we are keeping the import_shims
in Lilac in order to make any lingering old-style
import paths obvious to operators.

Update the ADR to reflect this. Also, add a table
mapping old import paths to new ones.
This commit is contained in:
Kyle McCormick
2021-01-06 09:54:12 -05:00
committed by Kyle McCormick
parent 2b5d916512
commit dfa60a5e92

View File

@@ -5,10 +5,12 @@ It used to be acceptable to import code from "lms.djangoapps.something" with `im
Soon the old import style will stop working.
Status
======
In Progress
Complete (as of the Lilac Open edX release)
Context
=======
@@ -32,7 +34,7 @@ This deprecation will take place in the following steps:
4. Fix all instances where the ``sys.path``-based modules were ``patch``-ed in unit tests, as those patches no longer work.
5. Once all instances of the ``sys.path``-based imports have been migrated in the ``edx-platform`` codebase, officially deprecate that import pattern for the next open-source release (as per the standard deprecation process).
5. Once all instances of the ``sys.path``-based imports have been migrated in the ``edx-platform`` codebase, officially deprecate that import pattern for the next open-source release.
6. Monitor logs to address continued use of the ``sys.path`` based import patterns.
@@ -40,3 +42,182 @@ Goals
=====
Eliminate modifications to ``sys.path`` to enable easier tool use in edx-platform.
Timeline
========
In the Koa release, usages of the old import style raised instances of
``DeprecatedEdxPlatformImportWarning``, but still worked.
In the Lilac release, usages of old import paths will raise instances of
``DeprecatedEdxPlatformImportError``, breaking any code that was not updated.
This error class is intentionally *not* a subclass of ``ImportError``;
to understand why, consider the following common pattern used in packages outside
of the edx-platform repo itself::
try:
from student.models import CourseEnrollment
except ImportError:
CourseEnrollment = None
This pattern is (unfortunately) widely used to so packages can import
objects from edx-platform modules in developer or production environments,
whilst falling back to ``None`` when unit tests are run.
Now, if the the above old-style import statement raised a subclass of
``ImportError``, then it would be silently caught by the ``except`` clause!
By instead raising from a disjoint error class in the Lilac release,
we make it more likely that operators can quickly notice and fix
any lingering old-style import paths as they migrate from Koa.
In Maple and beyond, old import paths will simply raise ``ImportError``,
allowing us to remove the ``import_shims`` directory tree.
Upgrade Guide
=============
Where to look for old import paths:
* Forks of ``edx-platform`` itself.
* Packages that import edx-platform code (``edx-completion``, ``edx-enterprise``, et al).
* Repositories/files that override Ansible variables or ``edx-platform`` Django settings.
What forms old import paths may take:
* Direct imports (e.g. ``import courseware.views``)
* Direct "from" imports (e.g. ``from contentstore.models import VideoUploadConfig``)
* String references to modules (e.g. ``@patch('edxmako.LOOKUP', {})``)
* YAML references to modules (e.g. ``KEY_FUNCTION: util.memcache.safe_key``)
What the old imports are, and their replacements:
+-------------------------------+----------------------------------------------+
+ **Old prefix** | **New prefix** |
+-------------------------------+----------------------------------------------+
| ``badges`` | ``lms.djangoapps.badges`` |
+-------------------------------+----------------------------------------------+
| ``branding`` | ``lms.djangoapps.branding`` |
+-------------------------------+----------------------------------------------+
| ``bulk_email`` | ``lms.djangoapps.bulk_email`` |
+-------------------------------+----------------------------------------------+
| ``bulk_enroll`` | ``lms.djangoapps.bulk_enroll`` |
+-------------------------------+----------------------------------------------+
| ``ccx`` | ``lms.djangoapps.ccx`` |
+-------------------------------+----------------------------------------------+
| ``certificates`` | ``lms.djangoapps.certificates`` |
+-------------------------------+----------------------------------------------+
| ``commerce`` | ``lms.djangoapps.commerce`` |
+-------------------------------+----------------------------------------------+
| ``course_api`` | ``lms.djangoapps.course_api`` |
+-------------------------------+----------------------------------------------+
| ``course_blocks`` | ``lms.djangoapps.course_blocks`` |
+-------------------------------+----------------------------------------------+
| ``course_goals`` | ``lms.djangoapps.course_goals`` |
+-------------------------------+----------------------------------------------+
| ``course_home_api`` | ``lms.djangoapps.course_home_api`` |
+-------------------------------+----------------------------------------------+
| ``courseware`` | ``lms.djangoapps.courseware`` |
+-------------------------------+----------------------------------------------+
| ``coursewarehistoryextended`` | ``lms.djangoapps.coursewarehistoryextended`` |
+-------------------------------+----------------------------------------------+
| ``course_wiki`` | ``lms.djangoapps.course_wiki`` |
+-------------------------------+----------------------------------------------+
| ``dashboard`` | ``lms.djangoapps.dashboard`` |
+-------------------------------+----------------------------------------------+
| ``debug`` | ``lms.djangoapps.debug`` |
+-------------------------------+----------------------------------------------+
| ``discussion`` | ``lms.djangoapps.discussion`` |
+-------------------------------+----------------------------------------------+
| ``edxnotes`` | ``lms.djangoapps.edxnotes`` |
+-------------------------------+----------------------------------------------+
| ``email_marketing`` | ``lms.djangoapps.email_marketing`` |
+-------------------------------+----------------------------------------------+
| ``experiments`` | ``lms.djangoapps.experiments`` |
+-------------------------------+----------------------------------------------+
| ``gating`` | ``lms.djangoapps.gating`` |
+-------------------------------+----------------------------------------------+
| ``grades`` | ``lms.djangoapps.grades`` |
+-------------------------------+----------------------------------------------+
| ``instructor`` | ``lms.djangoapps.instructor`` |
+-------------------------------+----------------------------------------------+
| ``instructor_analytics`` | ``lms.djangoapps.instructor_analytics`` |
+-------------------------------+----------------------------------------------+
| ``instructor_task`` | ``lms.djangoapps.instructor_task`` |
+-------------------------------+----------------------------------------------+
| ``learner_dashboard`` | ``lms.djangoapps.learner_dashboard`` |
+-------------------------------+----------------------------------------------+
| ``lms_initialization`` | ``lms.djangoapps.lms_initialization`` |
+-------------------------------+----------------------------------------------+
| ``lms_xblock`` | ``lms.djangoapps.lms_xblock`` |
+-------------------------------+----------------------------------------------+
| ``lti_provider`` | ``lms.djangoapps.lti_provider`` |
+-------------------------------+----------------------------------------------+
| ``mailing`` | ``lms.djangoapps.mailing`` |
+-------------------------------+----------------------------------------------+
| ``mobile_api`` | ``lms.djangoapps.mobile_api`` |
+-------------------------------+----------------------------------------------+
| ``monitoring`` | ``lms.djangoapps.monitoring`` |
+-------------------------------+----------------------------------------------+
| ``program_enrollments`` | ``lms.djangoapps.program_enrollments`` |
+-------------------------------+----------------------------------------------+
| ``rss_proxy`` | ``lms.djangoapps.rss_proxy`` |
+-------------------------------+----------------------------------------------+
| ``shoppingcart`` | ``lms.djangoapps.shoppingcart`` |
+-------------------------------+----------------------------------------------+
| ``staticbook`` | ``lms.djangoapps.staticbook`` |
+-------------------------------+----------------------------------------------+
| ``static_template_view`` | ``lms.djangoapps.static_template_view`` |
+-------------------------------+----------------------------------------------+
| ``support`` | ``lms.djangoapps.support`` |
+-------------------------------+----------------------------------------------+
| ``survey`` | ``lms.djangoapps.survey`` |
+-------------------------------+----------------------------------------------+
| ``teams`` | ``lms.djangoapps.teams`` |
+-------------------------------+----------------------------------------------+
| ``tests`` | ``lms.djangoapps.tests`` |
+-------------------------------+----------------------------------------------+
| ``verify_student`` | ``lms.djangoapps.verify_student`` |
+-------------------------------+----------------------------------------------+
| ``course_action_state`` | ``common.djangoapps.course_action_state`` |
+-------------------------------+----------------------------------------------+
| ``course_modes`` | ``common.djangoapps.course_modes`` |
+-------------------------------+----------------------------------------------+
| ``database_fixups`` | ``common.djangoapps.database_fixups`` |
+-------------------------------+----------------------------------------------+
| ``edxmako`` | ``common.djangoapps.edxmako`` |
+-------------------------------+----------------------------------------------+
| ``entitlements`` | ``common.djangoapps.entitlements`` |
+-------------------------------+----------------------------------------------+
| ``pipeline_mako`` | ``common.djangoapps.pipeline_mako`` |
+-------------------------------+----------------------------------------------+
| ``static_replace`` | ``common.djangoapps.static_replace`` |
+-------------------------------+----------------------------------------------+
| ``status`` | ``common.djangoapps.status`` |
+-------------------------------+----------------------------------------------+
| ``student`` | ``common.djangoapps.student`` |
+-------------------------------+----------------------------------------------+
| ``terrain`` | ``common.djangoapps.terrain`` |
+-------------------------------+----------------------------------------------+
| ``third_party_auth`` | ``common.djangoapps.third_party_auth`` |
+-------------------------------+----------------------------------------------+
| ``track`` | ``common.djangoapps.track`` |
+-------------------------------+----------------------------------------------+
| ``util`` | ``common.djangoapps.util`` |
+-------------------------------+----------------------------------------------+
| ``xblock_django`` | ``common.djangoapps.xblock_django`` |
+-------------------------------+----------------------------------------------+
| ``api`` | ``cms.djangoapps.api`` |
+-------------------------------+----------------------------------------------+
| ``cms_user_tasks`` | ``cms.djangoapps.cms_user_tasks`` |
+-------------------------------+----------------------------------------------+
| ``contentstore`` | ``cms.djangoapps.contentstore`` |
+-------------------------------+----------------------------------------------+
| ``course_creators`` | ``cms.djangoapps.course_creators`` |
+-------------------------------+----------------------------------------------+
| ``maintenance`` | ``cms.djangoapps.maintenance`` |
+-------------------------------+----------------------------------------------+
| ``models`` | ``cms.djangoapps.models`` |
+-------------------------------+----------------------------------------------+
| ``pipeline_js`` | ``cms.djangoapps.pipeline_js`` |
+-------------------------------+----------------------------------------------+
| ``xblock_config`` | ``cms.djangoapps.xblock_config`` |
+-------------------------------+----------------------------------------------+