Rename sys_path_hacks/ to import_shims/ and silence DeprecatedEdxPlatformImportWarning by default #25458
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kdmccormick
force-pushed
the
kdmccormick/sys-path-hacks-rename-to-import-shims
branch
2 times, most recently
from
e32e321 to
077ef47
Compare
kdmccormick
changed the title
kdmccormick
force-pushed
the
kdmccormick/sys-path-hacks-rename-to-import-shims
branch
2 times, most recently
from
086c470 to
1593e4e
Compare
|
fyi @cpennington |
kdmccormick
force-pushed
the
kdmccormick/sys-path-hacks-rename-to-import-shims
branch
from
1593e4e to
4b88c94
Compare
cpennington
approved these changes
Oct 28, 2020
kdmccormick
force-pushed
the
kdmccormick/sys-path-hacks-rename-to-import-shims
branch
from
4b88c94 to
6d39491
Compare
|
Planning to roll this out tomorrow morning or when the tasks issues quiet down, whichever comes later. |
2 tasks
The old folder name is somewhat confusing, because the folder contains shims to _compensate for the removal of sys.path hacks_, but does not contain the sys.path hacks themselves. Furthermore, this import_shims/ system could also be used for other import path changes, such as turning the locally-installed packages in common/lib/ into regular, importable modules (e.g. `from common.lib.xmodule import abc` instead of `from xmodule import abc`). So, a name that is not specific to the sys.path hacks may be better in the medium-to-long term. Along the same lines, we also rename SysPathHackWarning to DeprecatedEdxPlatformImportWarning.
Command: import_shims/generate_shims.sh lms/djangoapps import_shims/lms
Command: import_shims/generate_shims.sh cms/djangoapps import_shims/studio
kdmccormick
force-pushed
the
kdmccormick/sys-path-hacks-rename-to-import-shims
branch
from
6d39491 to
edc3ab1
Compare
|
Your PR has finished running tests. There were no failures. |
kdmccormick
deleted the
kdmccormick/sys-path-hacks-rename-to-import-shims
branch
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes 2300 files, but only changes like 7 of them interestingly. For reviewers, I recommend actually reviewing commits 1 and 2, and then just briefly looking at commits 3-5 to get an idea of what they do.
Commit 1: Rename sys_path_hacks/ to import_shims/
The old folder name is somewhat confusing, because the
folder contains shims to compensate for the removal
of sys.path hacks, but does not contain the sys.path
hacks themselves.
Furthermore, this import_shims/ system could also be
used for other import path changes, such as turning
the locally-installed packages in common/lib/
into regular, importable modules
(e.g.
from common.lib.xmodule import abcinstead offrom xmodule import abc). So, a name that is notspecific to the sys.path hacks may be better
in the medium-to-long term.
Along the same lines, we also rename SysPathHackWarning
to DeprecatedEdxPlatformImportWarning.
Commit 2: Silence DeprecatedEdxPlatformImportWarning by default
The influx of sys.path-hack-removal warnings is causing developers pain, as every test run and LMS/Studio startup is accompanied by ~200 warning messages that are generally irrelevant to most developers. I have a PR to actually fix most of those warnings, but it needs time for review and thoughtful roll-out. This commit will shut up the warnings by default in the mean time.
Commits 3-5: Rename sys_path_hacks module tree, regenerate LMS modules, and regenerate Studio modules
These are in separate commits to make the interesting diffs (commits 1 and 2) more readable.
Post-merge follow-up
Send email instructing devs to remove the SysPathHackWarning lines from private.py
Next up