Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat!: change names of dynamic MFE config Django settings

Browse Source 
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.
This commit is contained in:
Kyle McCormick
2022-08-05 16:34:00 -04:00
committed by Kyle McCormick
parent 0ff6d50734
commit 8edefe74ff
5 changed files with 105 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
lms/djangoapps/mfe_config_api/docs/decisions/0001-mfe-config-api.rst
View File

@@ -18,7 +18,7 @@ Decision
- A lightweight API will be created that returns the mfe configuration variables from the site configuration or django settings. `PR Discussion about django settings`_
- The API will be enabled or disabled using the setting ``ENABLE_MFE_CONFIG_API``.
- The API will take the mfe configuration in the ``MFE_CONFIG`` keyset in the site configuration (admin > site configuration > your domain) or in django settings.
- This API allows to consult the configurations by specific MFE. Making a request like ``api/v1/mfe_config?mfe=mymfe`` will return the configuration defined in ``MFE_CONFIG_MYMFE`` merged with the ``MFE_CONFIG`` configuration.
- This API allows to consult the configurations by specific MFE. Making a request like ``/api/v1/mfe_config?mfe=mymfe`` will return the configuration defined in ``MFE_CONFIG_OVERRIDES["mymfe"]`` merged with the ``MFE_CONFIG`` configuration.
- The API will have a mechanism to cache the response with ``MFE_CONFIG_API_CACHE_TIMEOUT`` variable.
- The API will live in lms/djangoapps because this is not something Studio needs to serve and it is a lightweight API. `PR Discussion`_
- The API will not require authentication or authorization.

97
lms/djangoapps/mfe_config_api/tests/test_views.py
View File

@@ -27,14 +27,14 @@ class MFEConfigTestCase(APITestCase):
Expected result:
- The get_value method of the configuration_helpers in the views is called once with the
parameters ("MFE_CONFIG", getattr(settings, "MFE_CONFIG", {})).
parameters ("MFE_CONFIG", settings.MFE_CONFIG)
- The status of the response of the request is a HTTP_200_OK.
- The json of the response of the request is equal to the mocked configuration.
"""
configuration_helpers_mock.get_value.return_value = {"EXAMPLE_VAR": "value"}
response = self.client.get(self.mfe_config_api_url)
configuration_helpers_mock.get_value.assert_called_once_with("MFE_CONFIG", getattr(settings, "MFE_CONFIG", {}))
configuration_helpers_mock.get_value.assert_called_once_with("MFE_CONFIG", settings.MFE_CONFIG)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json(), {"EXAMPLE_VAR": "value"})
@@ -44,57 +44,87 @@ class MFEConfigTestCase(APITestCase):
Expected result:
- The get_value method of the configuration_helpers in the views is called twice, once with the
parameters ("MFE_CONFIG", getattr(settings, "MFE_CONFIG", {})) and once with the parameters
("MFE_CONFIG_MYMFE", getattr(settings, "MFE_CONFIG_MYMFE", {})).
and one for get_value("MFE_CONFIG_MYMFE", getattr(settings, "MFE_CONFIG_MYMFE", {})).
parameters ("MFE_CONFIG", settings.MFE_CONFIG)
and once with the parameters ("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES).
- The json of the response is the merge of both mocked configurations.
"""
configuration_helpers_mock.get_value.side_effect = [{"EXAMPLE_VAR": "value", "OTHER": "other"},
{"EXAMPLE_VAR": "mymfe_value"}]
configuration_helpers_mock.get_value.side_effect = [
{"EXAMPLE_VAR": "value", "OTHER": "other"},
{"mymfe": {"EXAMPLE_VAR": "mymfe_value"}},
]
response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe")
self.assertEqual(response.status_code, status.HTTP_200_OK)
calls = [call("MFE_CONFIG", getattr(settings, "MFE_CONFIG", {})),
call("MFE_CONFIG_MYMFE", getattr(settings, "MFE_CONFIG_MYMFE", {}))]
calls = [call("MFE_CONFIG", settings.MFE_CONFIG),
call("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES)]
configuration_helpers_mock.get_value.assert_has_calls(calls)
self.assertEqual(response.json(), {"EXAMPLE_VAR": "mymfe_value", "OTHER": "other"})
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
@ddt.data(
[{}, {}, {}],
[{"EXAMPLE_VAR": "value"}, {}, {"EXAMPLE_VAR": "value"}],
[{}, {"EXAMPLE_VAR": "mymfe_value"}, {"EXAMPLE_VAR": "mymfe_value"}],
[{"EXAMPLE_VAR": "value"}, {"EXAMPLE_VAR": "mymfe_value"}, {"EXAMPLE_VAR": "mymfe_value"}],
[{"EXAMPLE_VAR": "value", "OTHER": "other"}, {"EXAMPLE_VAR": "mymfe_value"},
{"EXAMPLE_VAR": "mymfe_value", "OTHER": "other"}],
)
@ddt.unpack
@ddt.data(
dict(
mfe_config={},
mfe_config_overrides={},
expected_response={},
),
dict(
mfe_config={"EXAMPLE_VAR": "value"},
mfe_config_overrides={},
expected_response={"EXAMPLE_VAR": "value"},
),
dict(
mfe_config={},
mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}},
expected_response={"EXAMPLE_VAR": "mymfe_value"},
),
dict(
mfe_config={"EXAMPLE_VAR": "value"},
mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}},
expected_response={"EXAMPLE_VAR": "mymfe_value"},
),
dict(
mfe_config={"EXAMPLE_VAR": "value", "OTHER": "other"},
mfe_config_overrides={"mymfe": {"EXAMPLE_VAR": "mymfe_value"}},
expected_response={"EXAMPLE_VAR": "mymfe_value", "OTHER": "other"},
),
dict(
mfe_config={"EXAMPLE_VAR": "value"},
mfe_config_overrides={"yourmfe": {"EXAMPLE_VAR": "yourmfe_value"}},
expected_response={"EXAMPLE_VAR": "value"},
),
dict(
mfe_config={"EXAMPLE_VAR": "value"},
mfe_config_overrides={
"yourmfe": {"EXAMPLE_VAR": "yourmfe_value"},
"mymfe": {"EXAMPLE_VAR": "mymfe_value"},
},
expected_response={"EXAMPLE_VAR": "mymfe_value"},
),
)
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
def test_get_mfe_config_with_queryparam_multiple_configs(
self,
configuration_helpers_mock,
mfe_config,
mfe_config_mymfe,
mfe_config_overrides,
expected_response,
configuration_helpers_mock
):
"""Test the get mfe config with a query param and different settings in mfe_config and mfe_config_mfe inside
"""Test the get mfe config with a query param and different settings in mfe_config and mfe_config_overrides with
the site configuration to test that the merge of the configurations is done correctly and mymfe config take
precedence.
In the ddt data the following structure is being passed:
[mfe_config, mfe_config_mymfe, expected_response]
Expected result:
- The get_value method of the configuration_helpers in the views is called twice, once with the
parameters ("MFE_CONFIG", getattr(settings, "MFE_CONFIG", {})) and once with the parameters
("MFE_CONFIG_MYMFE", getattr(settings, "MFE_CONFIG_MYMFE", {})).
parameters ("MFE_CONFIG", settings.MFE_CONFIG)
and once with the parameters ("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES).
- The json of the response is the expected_response passed by ddt.data.
"""
configuration_helpers_mock.get_value.side_effect = [mfe_config, mfe_config_mymfe]
configuration_helpers_mock.get_value.side_effect = [mfe_config, mfe_config_overrides]
response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe")
self.assertEqual(response.status_code, status.HTTP_200_OK)
calls = [call("MFE_CONFIG", getattr(settings, "MFE_CONFIG", {})),
call("MFE_CONFIG_MYMFE", getattr(settings, "MFE_CONFIG_MYMFE", {}))]
calls = [call("MFE_CONFIG", settings.MFE_CONFIG),
call("MFE_CONFIG_OVERRIDES", settings.MFE_CONFIG_OVERRIDES)]
configuration_helpers_mock.get_value.assert_has_calls(calls)
self.assertEqual(response.json(), expected_response)
@@ -106,20 +136,19 @@ class MFEConfigTestCase(APITestCase):
- The json response is equal to MFE_CONFIG in lms/envs/test.py"""
response = self.client.get(self.mfe_config_api_url)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json(), getattr(settings, "MFE_CONFIG", {}))
self.assertEqual(response.json(), settings.MFE_CONFIG)
def test_get_mfe_config_with_queryparam_from_django_settings(self):
"""Test that when there is no site configuration, the API with queryparam takes the django settings.
Expected result:
- The status of the response of the request is a HTTP_200_OK.
- The json response is equal to MFE_CONFIG merged with MFE_CONFIG_MYMFE in lms/envs/test.py
- The json response is equal to MFE_CONFIG merged with MFE_CONFIG_OVERRIDES['mymfe']
"""
response = self.client.get(f"{self.mfe_config_api_url}?mfe=mymfe")
self.assertEqual(response.status_code, status.HTTP_200_OK)
expected_response = getattr(settings, "MFE_CONFIG", {})
expected_response.update(getattr(settings, "MFE_CONFIG_MYMFE", {}))
self.assertEqual(response.json(), expected_response)
expected = {**settings.MFE_CONFIG, **settings.MFE_CONFIG_OVERRIDES["mymfe"]}
self.assertEqual(response.json(), expected)
@patch("lms.djangoapps.mfe_config_api.views.configuration_helpers")
@override_settings(ENABLE_MFE_CONFIG_API=False)

12
lms/djangoapps/mfe_config_api/views.py
View File

@@ -43,10 +43,12 @@ class MFEConfigView(APIView):
if not settings.ENABLE_MFE_CONFIG_API:
return HttpResponseNotFound()
mfe_config = configuration_helpers.get_value('MFE_CONFIG', getattr(settings, 'MFE_CONFIG', {}))
mfe_config = configuration_helpers.get_value('MFE_CONFIG', settings.MFE_CONFIG)
if request.query_params.get('mfe'):
mfe = str(request.query_params.get('mfe')).upper()
mfe_config.update(configuration_helpers.get_value(
f'MFE_CONFIG_{mfe}', getattr(settings, f'MFE_CONFIG_{mfe}', {})))
mfe = str(request.query_params.get('mfe'))
app_config = configuration_helpers.get_value(
'MFE_CONFIG_OVERRIDES',
settings.MFE_CONFIG_OVERRIDES,
)
mfe_config.update(app_config.get(mfe, {}))
return JsonResponse(mfe_config, status=status.HTTP_200_OK)

29
lms/envs/common.py
View File

@@ -5171,8 +5171,10 @@ ENABLE_MFE_CONFIG_API = False
# .. setting_name: MFE_CONFIG
# .. setting_implementation: DjangoSetting
# .. setting_default: {}
# .. setting_description: Is a configuration that will be exposed by the MFE Config API to be consumed by the mfes
# Example: {
# .. setting_description: Is a configuration that will be exposed by the MFE Config API to be consumed by the MFEs.
# Contains configuration common to all MFEs. When a specific MFE's configuration is requested, these values
# will be treated as a base and then overriden/supplemented by those in `MFE_CONFIG_OVERRIDES`.
# Example: {
# "BASE_URL": "https://name_of_mfe.example.com",
# "LANGUAGE_PREFERENCE_COOKIE_NAME": "example-language-preference",
# "CREDENTIALS_BASE_URL": "https://credentials.example.com",
@@ -5182,11 +5184,30 @@ ENABLE_MFE_CONFIG_API = False
# "LOGOUT_URL": "https://courses.example.com/logout",
# "STUDIO_BASE_URL": "https://studio.example.com",
# "LOGO_URL": "https://courses.example.com/logo.png"
# }
# }
# .. setting_use_cases: open_edx
# .. setting_creation_date: 2022-07-08
# .. setting_creation_date: 2022-08-05
MFE_CONFIG = {}
# .. setting_name: MFE_CONFIG_OVERRIDES
# .. setting_implementation: DjangoSetting
# .. setting_default: {}
# .. setting_description: Overrides or additions to `MFE_CONFIG` for when a specific MFE is requested
# by the MFE Config API. Top-level keys are APP_IDs, a.k.a. the name of the MFE (for example,
# for an MFE named "frontend-app-xyz", the top-level key would be "xyz").
# Example: {
# "gradebook": {
# "BASE_URL": "https://gradebook.example.com",
# },
# "profile": {
# "BASE_URL": "https://profile.example.com",
# "ENABLE_LEARNER_RECORD_MFE": "true",
# },
# }
# .. setting_use_cases: open_edx
# .. setting_creation_date: 2022-08-05
MFE_CONFIG_OVERRIDES = {}
# .. setting_name: MFE_CONFIG_API_CACHE_TIMEOUT
# .. setting_default: 60*5
# .. setting_description: The MFE Config API response will be cached during the

12
lms/envs/test.py
View File

@@ -656,7 +656,13 @@ MFE_CONFIG = {
"LOGO_URL": "https://courses.example.com/logo.png"
}
MFE_CONFIG_MYMFE = {
"LANGUAGE_PREFERENCE_COOKIE_NAME": "mymfe-language-preference",
"LOGO_URL": "https://courses.example.com/mymfe-logo.png"
MFE_CONFIG_OVERRIDES = {
"mymfe": {
"LANGUAGE_PREFERENCE_COOKIE_NAME": "mymfe-language-preference",
"LOGO_URL": "https://courses.example.com/mymfe-logo.png",
},
"yourmfe": {
"LANGUAGE_PREFERENCE_COOKIE_NAME": "yourmfe-language-preference",
"LOGO_URL": "https://courses.example.com/yourmfe-logo.png",
},
}