Skip to content

Extracting plugin app from edx-platform - #24678

Merged
jinder1s merged 12 commits into
masterfrom
msingh/extract/plugins_infra
Aug 12, 2020
Merged

Extracting plugin app from edx-platform#24678
jinder1s merged 12 commits into
masterfrom
msingh/extract/plugins_infra

Conversation

@jinder1s

@jinder1s jinder1s commented Aug 4, 2020

Copy link
Copy Markdown
Contributor

ARCHBOM-1402
This PR extracts the code that enables plugins in edx-platform and puts it in edx-django-utils. This is done to allow other IDAS to add plugin functionality.

As of right now: this has been tested in sandbox and devstack. Things seem to be working.

@jinder1s

jinder1s commented Aug 4, 2020

Copy link
Copy Markdown
Contributor Author

jenkins run python

@jinder1s
jinder1s marked this pull request as ready for review August 7, 2020 18:52
Comment thread requirements/edx/base.in Outdated
edx-django-sites-extensions
edx-django-utils
# TODO(jinder): uncomment below after plugin code has been moved
# edx-django-utils

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ reviewer: I plan on uncommenting this once this has merged: openedx/edx-django-utils#48

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

  1. A lot of files were deleted, but I didn’t see references change. Are those all used local to plugins only?
  2. Do we want a short decision doc that points to the other one regarding moving plugins?

Comment thread cms/envs/production.py
Comment thread common/djangoapps/student/views/dashboard.py Outdated
Comment thread common/djangoapps/student/views/dashboard.py Outdated
Comment thread docs/guides/extension_points.rst Outdated
Comment thread docs/guides/extension_points.rst Outdated
Comment thread openedx/core/djangoapps/plugins/README.rst
Comment thread openedx/core/djangoapps/plugins/apps.py Outdated
Comment thread openedx/core/djangoapps/plugins/constants.py Outdated
@jinder1s

Copy link
Copy Markdown
Contributor Author

@robrap

A lot of files were deleted, but I didn’t see references change. Are those all used local to plugins only?

I've caught all the references to deleted files that I could find and as far as I can tell, the only file not referenced in edx_platform is .../plugins/utils.py, which is used internally by other deleted files.

Do we want a short decision doc that points to the other one regarding moving plugins?

This is a good idea. Will create one.

@robrap

robrap commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

We need a task for someone to do a local search through all repos for the following, to ensure we won't break anything:

openedx.core.lib.plugins
openedx.core.djangoapps.plugins

* Moving code
* changing imports to point to edx-django-utils
* make upgrade
* Adding documentation
@jinder1s
jinder1s force-pushed the msingh/extract/plugins_infra branch from 8cf41fa to 9ec4753 Compare August 10, 2020 14:29
jinder1s added 3 commits August 10, 2020 11:07
The now point to open api in edx_django_utils.plugins instead of edx_django_utils.plugins.{module_name}
Comment thread cms/envs/common.py Outdated
Comment thread requirements/edx/base.txt Outdated
edx-django-release-util==0.4.4 # via -r requirements/edx/base.in
edx-django-sites-extensions==2.5.1 # via -r requirements/edx/base.in
edx-django-utils==3.6.0 # via -r requirements/edx/base.in, django-config-models, edx-drf-extensions, edx-enterprise, edx-rest-api-client, edx-when
-e git+https://github.com/edx/edx-django-utils.git@msingh/add/fix#egg=edx-django-utils # via -r requirements/edx/base.in, django-config-models, edx-drf-extensions, edx-enterprise, edx-rest-api-client, edx-when

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ reviewer I will remove this link once both this PR and edx-django-utils PR is ready to be merged. I'm leaving it so that any change made based on reviewer comments can be tested easily.

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I tried to add "Nit" to those comments you can ignore, and not to those I'd like to see. I'll leave it up to you if you want to fix the Nits.

Comment thread cms/envs/common.py
Comment thread cms/envs/production.py
Comment thread docs/guides/extension_points.rst Outdated
Comment thread lms/djangoapps/program_enrollments/apps.py
Comment thread lms/envs/production.py
Comment thread openedx/core/djangoapps/user_authn/apps.py
Comment thread openedx/core/djangoapps/xblock/learning_context/manager.py Outdated
Comment thread openedx/core/djangoapps/zendesk_proxy/apps.py
Comment thread openedx/core/lib/course_tabs.py
Comment thread openedx/core/lib/tests/test_course_tab_api.py
Comment thread cms/envs/production.py
Comment thread lms/djangoapps/instructor/apps.py
Comment thread lms/envs/production.py
Comment thread lms/envs/test.py Outdated
Comment thread lms/urls.py
Comment thread openedx/core/djangoapps/xblock/learning_context/manager.py Outdated
Comment thread openedx/core/lib/course_tabs.py
Comment thread common/djangoapps/student/views/dashboard.py Outdated

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor fixes and then all set, once requirements are updated.

Comment thread docs/guides/extension_points.rst Outdated
Comment thread lms/djangoapps/grades/apps.py Outdated
@jinder1s

Copy link
Copy Markdown
Contributor Author

jenkins run all

1 similar comment
@jinder1s

Copy link
Copy Markdown
Contributor Author

jenkins run all

@edx-status-bot

Copy link
Copy Markdown

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

@jinder1s
jinder1s merged commit c76ed6a into master Aug 12, 2020
@jinder1s
jinder1s deleted the msingh/extract/plugins_infra branch August 12, 2020 11:48
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

kdmccormick added a commit that referenced this pull request Aug 12, 2020
This reverts commit c76ed6a.

This broke Pathways in LabXChange.
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants