Use full names for common.djangoapps imports; warn when using old style #25477

kdmccormick · 2020-10-29T13:38:11Z

This is of the sys.path modification removal & cleanup, started by https://github.com/edx/edx-platform/pull/24616 . See the ADR for details.

This PR fixes all imports of common/djangoapps code to use the preferred style (import common.djangoapps.myapp..) instead of the deprecated short version (import student...). It also adds those modules to import_shims/lms and import_shims/studio to provide partial and temporary support for the old, shorter import style. Warnings of the class DeprecatedEdxPlatformImportWarning are generated when the old style is used.

Related ARCH-BOM ticket: https://openedx.atlassian.net/browse/ARCHBOM-1550

kdmccormick · 2020-11-05T00:52:53Z

@nedbat FYI, I amended a silly mistake in the "fix pylint violations" commit and rebased. I did the same for the canary PR.

…ction_state

edx-status-bot · 2020-11-09T23:18:08Z

Your PR has finished running tests. There were no failures.

… old style (#25477)" This reverts commit 151bd13.

edx-pipeline-bot · 2020-11-10T15:12:30Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

edx-pipeline-bot · 2020-11-10T15:29:58Z

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/deploy_to_stage

kdmccormick · 2020-11-10T15:59:18Z

^ e2e failures should be fixed by https://github.com/edx/edx-platform/pull/25558

edx-pipeline-bot · 2020-11-10T18:12:49Z

EdX Release Notice: This PR has been deployed to the production environment.

edx-pipeline-bot · 2020-11-10T20:08:32Z

EdX Release Notice: This PR has been rolled back from the production environment.

kdmccormick · 2020-11-10T20:38:03Z

This may smooth the re-rollout: https://github.com/edx/edx-platform/pull/25566

When rolling out #25477, we dropped hundreds of email activation tasks due to the renaming of student.send_activation_email to common.djangoapps.student.send_activation_email, and lost more when we rolled that PR back. This happens because of blue/green deployment: old workers are still online for a while after deploying, so there is a period of time when the task names are mismatched. To prevent this from happening again, this will make it so the import changes don't change the names of any of the Celery tasks.

edx-pipeline-bot · 2020-11-11T00:25:43Z

EdX Release Notice: This PR has been deployed to the production environment.

regisb · 2020-11-24T10:26:29Z

FYI @kdmccormick there is a change in this PR which is causing pylint to crash. To reproduce, run the following command:

$ pylint manage.py
Traceback (most recent call last):
  File "/home/regis/venvs/openedx3.8/bin/pylint", line 8, in <module>
    sys.exit(run_pylint())
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/__init__.py", line 23, in run_pylint
    PylintRun(sys.argv[1:])
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/lint.py", line 1689, in __init__
    linter.load_config_file()
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 736, in load_config_file
    self.global_set_option(option, value)
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 629, in global_set_option
    self._all_options[opt].set_option(opt, value)
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/lint.py", line 729, in set_option
    checkers.BaseTokenChecker.set_option(self, optname, value, action, optdict)
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 825, in set_option
    value = _validate(value, optdict, optname)
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 234, in _validate
    return _call_validator(_type, optdict, name, value)
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 215, in _call_validator
    return VALIDATORS[opttype](optdict, option, value)
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 175, in _regexp_csv_validator
    return [_regexp_validator(_, name, val) for val in _csv_validator(_, name, value)]
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 175, in <listcomp>
    return [_regexp_validator(_, name, val) for val in _csv_validator(_, name, value)]
  File "/home/regis/venvs/openedx3.8/lib/python3.8/site-packages/pylint/config.py", line 170, in _regexp_validator
    return re.compile(value)
  File "/home/regis/install/Python-3.8.5/lib/python3.8/re.py", line 252, in compile
    return _compile(pattern, flags)
  File "/home/regis/install/Python-3.8.5/lib/python3.8/re.py", line 304, in _compile
    p = sre_compile.compile(pattern, flags)
  File "/home/regis/install/Python-3.8.5/lib/python3.8/sre_compile.py", line 764, in compile
    p = sre_parse.parse(p, flags)
  File "/home/regis/install/Python-3.8.5/lib/python3.8/sre_parse.py", line 948, in parse
    p = _parse_sub(source, state, flags & SRE_FLAG_VERBOSE, 0)
  File "/home/regis/install/Python-3.8.5/lib/python3.8/sre_parse.py", line 443, in _parse_sub
    itemsappend(_parse(source, state, verbose, nested + 1,
  File "/home/regis/install/Python-3.8.5/lib/python3.8/sre_parse.py", line 668, in _parse
    raise source.error("nothing to repeat",
re.error: nothing to repeat at position 0

The regexp that pylint chokes on is ignore-patterns = **/import_shims/**/*.py. I was able to get pylint to work by commenting out this line from pylintrc.

kdmccormick · 2020-11-24T12:25:13Z

@regisb Yeah, it looks like that ignore pattern fails when linting individual files. I have a fix ready for review in #25632 , which I was going to suggest should be cherry-picked into Koa.

kdmccormick · 2020-11-24T14:58:31Z

@regisb That PR I just linked is merged now; I can open a PR against koa.master later today.

regisb · 2020-11-24T15:07:13Z

That PR I just linked is merged now; I can open a PR against koa.master later today.

Yes, I think that would be great. Thanks @kdmccormick!

kdmccormick · 2020-11-24T21:17:21Z

Fix backport: https://github.com/edx/edx-platform/pull/25678

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477) (cherry picked from commit 472f0d0)

This was referenced Oct 29, 2020

Rename sys_path_hacks/ to import_shims/ and silence DeprecatedEdxPlatformImportWarning by default #25458

Merged

Use full names for lms.djangoapps imports #25401

Merged

kdmccormick force-pushed the kdmccormick/sys-path-hacks-common branch 6 times, most recently from 03b9a3c to 9d3f646 Compare November 4, 2020 15:01

kdmccormick changed the title ~~[WIP] Stop appending common/djangoapps to sys.path~~ Use full names for common.djangoapps imports; warn when using old style Nov 4, 2020

kdmccormick mentioned this pull request Nov 4, 2020

[nomerge] Test build of "Use full names for common.djangoapps imports; warn when using old style" without import shims #25523

Closed

kdmccormick force-pushed the kdmccormick/sys-path-hacks-common branch 3 times, most recently from 1c0ab81 to 259768f Compare November 5, 2020 00:51

kdmccormick force-pushed the kdmccormick/sys-path-hacks-common branch from 259768f to e0b062f Compare November 9, 2020 14:57

kdmccormick mentioned this pull request Nov 9, 2020

Prefix edx-platform/common/djangoapps imports with 'common.djangoapps' openedx/edx-enterprise#1036

Merged

kdmccormick force-pushed the kdmccormick/sys-path-hacks-common branch 3 times, most recently from cc31f67 to e92a6e3 Compare November 9, 2020 17:27

kdmccormick marked this pull request as ready for review November 9, 2020 18:15

nedbat approved these changes Nov 9, 2020

View reviewed changes

kdmccormick added 9 commits November 9, 2020 17:24

Import from lms.djangoapps.experiments instead of experiments

d865c04

Generate common/djangoapps import shims for LMS

edad986

Generate common/djangoapps import shims for Studio

9b4a232

Stop appending common/djangoapps to sys.path

58e14d5

Import from common.djangoapps.course_action_state instead of course_a…

2c8f541

…ction_state

Import from common.djangoapps.course_modes instead of course_modes

36f370a

Import from common.djangoapps.database_fixups instead of database_fixups

2dd47d4

Import from common.djangoapps.edxmako instead of edxmako

cea5e11

Import from common.djangoapps.entitlements instead of entitlements

4d43f77

kdmccormick merged commit 151bd13 into master Nov 10, 2020

kdmccormick deleted the kdmccormick/sys-path-hacks-common branch November 10, 2020 12:02

kdmccormick added a commit that referenced this pull request Nov 10, 2020

Revert "Use full names for common.djangoapps imports; warn when using…

3b834ba

… old style (#25477)" This reverts commit 151bd13.

kdmccormick mentioned this pull request Nov 10, 2020

Revert "Use full names for common.djangoapps imports; warn when using… #25557

Closed

kdmccormick mentioned this pull request Nov 10, 2020

Fix import of common.djangoapps.util.date_utils #25558

Merged

kdmccormick mentioned this pull request Nov 10, 2020

Undo renames of common.djangoapps tasks #25566

Merged

morenol mentioned this pull request Nov 20, 2020

Fix DeprecatedEdxPlatformImportWarning mitodl/edx-sga#286

Merged

mrtmm referenced this pull request in mrtmm/hastexo-xblock Mar 25, 2021

Use full names for common.djangoapps imports from edx-platform

5f8dc53

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

mrtmm referenced this pull request in mrtmm/hastexo-xblock Mar 25, 2021

Use full names for common.djangoapps imports from edx-platform

6eebde4

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

mrtmm referenced this pull request in mrtmm/hastexo-xblock Mar 25, 2021

Use full path for common.djangoapps imports for edx-platform

ec93461

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

mrtmm referenced this pull request in mrtmm/hastexo-xblock Mar 26, 2021

Use full path for common.djangoapps imports for edx-platform

372c79b

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

mrtmm referenced this pull request in mrtmm/hastexo-xblock Mar 26, 2021

Use full path for common.djangoapps imports for edx-platform

573219f

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

mrtmm referenced this pull request in mrtmm/hastexo-xblock Mar 26, 2021

Use full path for common.djangoapps imports for edx-platform

9cd5c9a

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

mrtmm referenced this pull request in mrtmm/hastexo-xblock Mar 26, 2021

Use full path for common.djangoapps imports for edx-platform

472f0d0

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477)

fghaas referenced this pull request in fghaas/hastexo-xblock Mar 26, 2021

Use full path for common.djangoapps imports for edx-platform

36647de

The short way of defining imports will be deprecated. (https://github.com/edx/edx-platform/pull/25477) (cherry picked from commit 472f0d0)

bryanlandia mentioned this pull request Oct 4, 2021

Tracking log events for successful login (first and third-party auth) appsembler/edx-platform#1020

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use full names for common.djangoapps imports; warn when using old style #25477

Use full names for common.djangoapps imports; warn when using old style #25477

kdmccormick commented Oct 29, 2020 •

edited

Loading

kdmccormick commented Nov 5, 2020

edx-status-bot commented Nov 9, 2020

edx-pipeline-bot commented Nov 10, 2020

edx-pipeline-bot commented Nov 10, 2020

kdmccormick commented Nov 10, 2020

edx-pipeline-bot commented Nov 10, 2020

edx-pipeline-bot commented Nov 10, 2020

kdmccormick commented Nov 10, 2020

edx-pipeline-bot commented Nov 11, 2020

regisb commented Nov 24, 2020

kdmccormick commented Nov 24, 2020

kdmccormick commented Nov 24, 2020

regisb commented Nov 24, 2020

kdmccormick commented Nov 24, 2020

Use full names for common.djangoapps imports; warn when using old style #25477

Use full names for common.djangoapps imports; warn when using old style #25477

Conversation

kdmccormick commented Oct 29, 2020 • edited Loading

kdmccormick commented Nov 5, 2020

edx-status-bot commented Nov 9, 2020

edx-pipeline-bot commented Nov 10, 2020

edx-pipeline-bot commented Nov 10, 2020

kdmccormick commented Nov 10, 2020

edx-pipeline-bot commented Nov 10, 2020

edx-pipeline-bot commented Nov 10, 2020

kdmccormick commented Nov 10, 2020

edx-pipeline-bot commented Nov 11, 2020

regisb commented Nov 24, 2020

kdmccormick commented Nov 24, 2020

kdmccormick commented Nov 24, 2020

regisb commented Nov 24, 2020

kdmccormick commented Nov 24, 2020

kdmccormick commented Oct 29, 2020 •

edited

Loading