Remove sys.path modification for Studio; use sys_path_hacks instead

Also, tweak the un_sys_path.sh script.
This commit is contained in:
Kyle McCormick
2020-09-30 10:49:03 -04:00
committed by Kyle McCormick
parent 78eae80e38
commit 4847a18a66
6 changed files with 39 additions and 18 deletions

View File

@@ -456,8 +456,11 @@ COURSES_ROOT = ENV_ROOT / "data"
GITHUB_REPO_ROOT = ENV_ROOT / "data"
# TODO: Is this next line necessary?
sys.path.append(REPO_ROOT)
sys.path.append(PROJECT_ROOT / 'djangoapps')
# TODO: The next two path modifications will be removed in an upcoming Open edX release.
# See docs/decisions/0007-sys-path-modification-removal.rst
sys.path.append(REPO_ROOT / 'sys_path_hacks' / 'studio')
sys.path.append(COMMON_ROOT / 'djangoapps')
# For geolocation ip database

View File

@@ -26,7 +26,7 @@ This deprecation will take place in the following steps:
1. Add a new folder (``sys_path_hacks``) to isolate the code used for deprecation warnings.
2. For every module importable using the ``sys.path`` style (for instance, ``courseware``), duplicate that module structure into the ``sys_path_hacks/lms`` (or ``sys_path_hacks/studio``) directory. Each file in that directory should do a wild-card import of the corresponding ``lms.djangoapps.`` module, and should log a warning indicating where it was imported from. For example, in ``sys_path_hacks/lms/courseware/views/views.py``, it will wild-card import ``from lms.djangoapps.courseware.views.views import *``. The ``sys_path_hacks/clean.sh`` script will generate these files.
2. For every module importable using the ``sys.path`` style (for instance, ``courseware``), duplicate that module structure into the ``sys_path_hacks/lms`` (or ``sys_path_hacks/studio``) directory. Each file in that directory should do a wild-card import of the corresponding ``lms.djangoapps.`` module, and should log a warning indicating where it was imported from. For example, in ``sys_path_hacks/lms/courseware/views/views.py``, it will wild-card import ``from lms.djangoapps.courseware.views.views import *``. The ``sys_path_hacks/un_sys_path.sh`` script will generate these files.
3. The ``sys.path`` modification will be changed to point to ``sys_path_hacks/lms``, rather than ``lms/djangoapps``. At this point, any code that references the modules directly will trigger warnings with logging about where the imports were coming from (to drive future cleanup efforts). The warnings will be instances of ``SysPathHackWarning`` (subclass of ``DeprecationWarning``).

View File

@@ -714,8 +714,10 @@ NODE_MODULES_ROOT = REPO_ROOT / "node_modules"
DATA_DIR = COURSES_ROOT
# TODO: Remove the rest of the sys.path modification here and in cms/envs/common.py
# TODO: Is this next line necessary?
sys.path.append(REPO_ROOT)
# TODO: The next two path modifications will be removed in an upcoming Open edX release.
# See docs/decisions/0007-sys-path-modification-removal.rst
sys.path.append(REPO_ROOT / 'sys_path_hacks' / 'lms')
sys.path.append(COMMON_ROOT / 'djangoapps')

View File

@@ -71,7 +71,7 @@
ignore = ,.git,.tox,migrations,node_modules,.pycharm_helpers
persistent = yes
load-plugins = edx_lint.pylint,pylint_django,pylint_celery
init-hook = "import sys; sys.path.extend(['sys_path_hacks/lms', 'cms/djangoapps', 'common/djangoapps'])"
init-hook = "import sys; sys.path.extend(['sys_path_hacks/lms', 'sys_path_hacks/studio', 'common/djangoapps'])"
[MESSAGES CONTROL]
enable =
@@ -464,4 +464,4 @@ int-import-graph =
[EXCEPTIONS]
overgeneral-exceptions = Exception
# 7bfb536a0940715cbd9013a31f37e8c2fc09f16c
# 0981d49b0645874588dcf4423195239dd9e414b7

View File

@@ -1,7 +1,7 @@
# pylintrc tweaks for use with edx_lint.
[MASTER]
ignore+ = ,.git,.tox,migrations,node_modules,.pycharm_helpers
init-hook="import sys; sys.path.extend(['sys_path_hacks/lms', 'cms/djangoapps', 'common/djangoapps'])"
init-hook="import sys; sys.path.extend(['sys_path_hacks/lms', 'sys_path_hacks/studio', 'common/djangoapps'])"
[MESSAGES CONTROL]
disable+ =

View File

@@ -1,20 +1,32 @@
#!/usr/bin/env bash
# Example usage:
# ~/edx-platform> sys_path_hacks/un_sys_path.sh lms
# Writing sys_path_hacks/lms/.../xyz.py
# ....
# ~/edx-platform>
#
# Usage:
#
# sys_path_hacks/un_sys_path.sh SOURCE DESTINATION
#
# where SOURCE is the folder from which modules will be recursively copied
# and DESTINATION is the subfolder of `sys_path_hacks` in which they end up.
#
# For example:
#
# ~/edx-platform> sys_path_hacks/un_sys_path.sh common/djangoapps studio
#
# will mirror the packages structure of `common/djangoapps` within `sys_path_hacks/studio`.
# One would run this if they want to mimic the effect of adding 'common/djangoapps'
# to `sys.path` within Studio.
# Shellchecks recommends using search/replace instead of sed. It's fine as is.
# Shellcheck recommends using search/replace instead of sed. It's fine as is.
# shellcheck disable=SC2001
set -e
set -o pipefail
set -u
TARGET="$1"
for path in $(find "${TARGET}/djangoapps/" -name '*.py' | grep -v migrations); do
if [[ "$path" == "${TARGET}/djangoapps/__init__.py" ]]; then
SOURCE="$1"
PYTHON_SOURCE="${SOURCE/\//.}"
DESTINATION="$2"
for path in $(find "${SOURCE}/" -name '*.py' | grep -v migrations); do
if [[ "$path" == "${SOURCE}/__init__.py" ]]; then
# Skip unnecessary root __init__.py.
continue
fi
@@ -23,15 +35,19 @@ for path in $(find "${TARGET}/djangoapps/" -name '*.py' | grep -v migrations); d
# We've gone to prod with this excluded, and it hasn't been a problem.
continue
fi
new_path=$(echo "$path" | sed "s#${TARGET}/djangoapps/#sys_path_hacks/${TARGET}/#")
if [[ "$path" == "cms/djangoapps/contentstore/management/commands/import.py" ]]; then
# Also skip this file because its name is problematic for the sys path hack.
continue
fi
new_path=$(echo "$path" | sed "s#${SOURCE}/#sys_path_hacks/${DESTINATION}/#")
python_path=$(echo "$path" | sed "s#/#.#g" | sed "s#.py##" | sed "s#.__init__##")
old_python_path=$(echo "$python_path" | sed "s#${TARGET}.djangoapps.##")
old_python_path=$(echo "$python_path" | sed "s#${PYTHON_SOURCE}.##")
echo "Writing ${new_path}"
mkdir -p "$(dirname "$new_path")"
{
echo "from sys_path_hacks.warn import warn_deprecated_import"
echo
echo "warn_deprecated_import('${TARGET}.djangoapps', '${old_python_path}')"
echo "warn_deprecated_import('${PYTHON_SOURCE}', '${old_python_path}')"
echo
echo "from ${python_path} import *"
} > "$new_path"