Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

And/backport mfe config api #730

Merged

Conversation

andrey-canon
Copy link
Contributor

Description

This is a backport of multiple changes which add and modify the MFE config API

  1. feat: add mfe config api openedx/edx-platform#30473
  2. Suggested changes to the MFE Config API openedx/edx-platform#30830

Testing instructions

  1. Add this to your tenant settings
    "ENABLE_MFE_CONFIG_API": true,
    "MFE_CONFIG": {
        "LMS_BASE_URL": "https://edunext.stage.nelp.gov.sa"
    },
    "MFE_CONFIG_OVERRIDES": {
        "learning": {
            "is_new": "yes"
        }
    },
  1. Go to /api/mfe_config/v1
  2. to see the app configuration add the queryparam /api/mfe_config/v1?mfe=learning

You can test it in stage with this branch https://github.com/nelc/manifest-sagar/tree/and/backport_mfe_config_api

felipemontoya and others added 4 commits January 25, 2023 15:44
Formerly, the settings were:

* `MFE_CONFIG` for common config.
* `MFE_CONFIG_<APP_ID>` for app-specific overrides,
  with each app getting its own Django setting.

This commit changes it to:

* `MFE_CONFIG` for common config (unchanged)
* `MFE_CONFIG_OVERRIDES` for app-specific overrides,
  where each app gets a top-level key in the dictionary.

Why the change?

* We want common.py to have a complete list of overridable settings, as
  it helps operators reason about configuration and allows us to generate
  config documentation using toggle annotations. Dynamically generating
  setting names based on arbitrary APP_IDs makes this impossible.
* getattr(...) generally makes code more complicated bug prone. Tools
  like pylint and mypy cannot effectively analyze any code that uses
  dynamic attribute access.
No functional changes here. This just uses the edx_api_doc_tools package
to add some additional documentation to the new API. The documentation
can be read from the code, or viewed by visiting
http://<LMS_ROOT>/api-docs and searching for "mfe_config".
* This changes the API's path. The reasoning is that this is Version 1 of
  the mfe_config API, not Version 1 of the LMS's entire API, so the v1
  should come after mfe_config.
* Why does this matter? Firstly, consistency. Secondly, it affects our
  generated API documentation. If you visited
  https://courses.edx.org/api-docs, you could see that the API was
  listed under "v1" instead of "mfe_config".
@andrey-canon andrey-canon force-pushed the and/backport_mfe_config_api branch from f7e270b to 5ed2f00 Compare January 25, 2023 20:44
@andrey-canon andrey-canon merged commit f3dd749 into ednx-release/mango.master.nelp Jan 25, 2023
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.

3 participants