refactor: import common/lib/ modules from canonical locations#30533
refactor: import common/lib/ modules from canonical locations#30533kdmccormick merged 1 commit intomasterfrom
Conversation
Unfortunately, some code in edx-platform is imported relative to sub-projects instead of the repository root. The only three remaining instances of this are: * common/lib/xmodule/xmodule (imported as just 'xmodule') * common/lib/capa/capa (imported as just 'capa') * openedx/core/lib/xblock_builtin/xblock_discussion (imported as just 'xblock_discussion') For more details on the situation, see: https://openedx.atlassian.net/browse/BOM-2579 (public, but requires Atlassian account creation). We would like to get to a point where all edx-platform import paths match their folder paths, relative to the repo root. For now, though, all common/lib/capa and common/lib/xmodule code should be imported as just `from capa` and `from xmodule`, respectively. Importing using the full `common.lib.xmodule.xmodule...` path will often work, but it instantiates a second instance of all modules imported this way, which in the past has led to very difficult-to-diagnose bugs. It also confuses tooling such as import-linter, which we are trying to add to edx-platform (see https://openedx.atlassian.net/browse/BOM-2576)
kdmccormick
force-pushed
the
kdmccormick/common-lib-import-fixes
branch
from
5d34f5e to
1a3bcc6
Compare
feanil
left a comment
feanil
left a comment
There was a problem hiding this comment.
Will this cause issues if these items are imported from in plugins or extensions or is that unlikely?
|
@feanil Since we're just fixing how these are referenced, not how they're defined, I don't think this would have any impact on external references to |
|
I was thinking it might be a problem if there are external references that cause double instantiation but it sounds like that would be unlikely? |
|
This PR only changes a tiny fraction of xmodule references; by far the most common way to import these paths in edx-platform is |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
| # .. setting_description: Set the python package.module.function that is reponsible of | ||
| # calling the remote service in charge of jailed code execution | ||
| CODE_JAIL_REST_SERVICE_REMOTE_EXEC = 'common.lib.capa.capa.safe_exec.remote_exec.send_safe_exec_request_v0' | ||
| CODE_JAIL_REST_SERVICE_REMOTE_EXEC = 'capa.safe_exec.remote_exec.send_safe_exec_request_v0' |
There was a problem hiding this comment.
FYI @ericfab179 and @felipemontoya -- I don't think this will require any changes to your Tutor codejail plugin, but heads up just in case.
|
EdX Release Notice: This PR has been deployed to the production environment. |
3 participants
Description
Unfortunately, some code in edx-platform is imported
relative to sub-projects instead of the repository root.
The only three remaining instances of this are:
(imported as just 'xblock_discussion')
For more details on the situation, see:
https://openedx.atlassian.net/browse/BOM-2579
(public, but requires Atlassian account creation).
We would like to get to a point where all edx-platform
import paths match their folder paths, relative to the repo
root. For now, though, all common/lib/capa and common/lib/xmodule
code should be imported as just
from capaandfrom xmodule,respectively. Importing using the full
common.lib.xmodule.xmodule...path will often work, but it instantiates a second instance of all
modules imported this way, which in the past has led to very
difficult-to-diagnose bugs. It also confuses tooling such as
import-linter, which we are trying to add to edx-platform
(see https://openedx.atlassian.net/browse/BOM-2576)
Testing information
N/A
Deadline
Not urgent, but this blocks #27011.