ARCHBOM-1339: add ADRs for extracting waffle_utils (#24798)

- Add ADR for the extraction of waffle_utils to
edx-toggles to reuse across IDAs.
- Add ADR for temporarily leaving CourseWaffleFlag
in edx-platform in order to simplify extraction.
- Add ADR for simplifying the interface for
WaffleFlag and WaffleSwitch by removing the
Namespace classes.

ARCHBOM-1339

Co-authored-by: Feanil Patel <feanil@edx.org>
This commit is contained in:
Robert Raposa
2020-09-25 15:02:46 -04:00
committed by GitHub
parent a04690d051
commit 1cc4ae2080
3 changed files with 104 additions and 0 deletions

View File

@@ -0,0 +1,35 @@
Waffle Utils Extraction
***********************
Status
======
Accepted
Context
=======
The waffle utilities in this app were created in edx-platform, but are generally useful across IDAs.
Decision
========
These utilities will be be moved to `edx/edx-toggles`_ so that they can be used by other IDAs. Additionally, the shared library will use the module name ``toggles``, rather than ``waffle_utils``, so it can more generally include non-waffle based toggle utilities as well.
.. _edx/edx-toggles: https://github.com/edx/edx-toggles
Consequences
============
* Rollout plan required to deprecate and update class references.
* See ADR 0003-leave-course-waffle-flag for the decision to leave the CourseWaffleFlag behind.
* See ADR 0004-waffle-util-namespacing for decision to change namespacing implementation before extraction.
* The toggle state endpoint, which is meant to be a Django Plugin, could be extracted as a separate step. This requires some additional work:
* Finishing out work around Django Plugin capabilities in edx-django-utils.
* Adding ability to document CourseWaffleFlag from edx-platform. Note: we may lose the ability to find course override data for toggles no longer in use, by looping through the entire model, unless we add a hook for this from the edx-toggles version.
* The helper `get_instance_module_name`_ should probably move to `edx_django_utils/monitoring/code_owner`_. It could be considered hacky, but is quite useful. It needs to work whether the class definition is in a library or an IDA, and whether the instance declaration is in a library or an IDA.
.. _get_instance_module_name: https://github.com/edx/edx-platform/blob/a8c3413a32510dc45301d0c462bf706a5f7ba487/openedx/core/djangoapps/waffle_utils/__init__.py#L521
.. _edx_django_utils/monitoring/code_owner: https://github.com/edx/edx-django-utils/tree/master/edx_django_utils/monitoring/code_owner

View File

@@ -0,0 +1,31 @@
Leave CourseWaffleFlag
**********************
Status
======
Accepted
Context
=======
It was decided in 0002-waffle-utils-extraction to remove waffle_utils to the shared library edx-toggles. However, moving the class CourseWaffleFlag would be complicated due to its model data, and it is unclear how much usage it would get in other IDAs.
Decision
========
It has been decided to leave CourseWaffleFlag inside edx-platform for the time being, and to delay its extraction until the work seems warranted (i.e. another IDA actually wants to make use of it).
Consequences
============
* The toggle state endpoint will need to be updated to allow WaffleFlag subclasses to add state data.
Rejected Alternative
====================
The alternative would be to extract CourseWaffleFlag to the shared library. As noted, this can be decided in the future. If extraction were to be pursued, some things to consider would be:
* Ensuring course override data is properly migrated from the old to new method of defining course overrides. It is possible that this migration would need to be documented and maintained for the Open edX release as well.
* Given the data migration needed, it might be practical to migrate course override data from a ConfigurationModel to a Django Setting (using remote config) before extraction.
* The toggle state report uses the term `course_id` for the course overrides, but this is actually a `course_run_id` in the context of other IDAs. Will its name be configurable per IDA?

View File

@@ -0,0 +1,38 @@
Waffle Util Namespacing
***********************
Status
======
Accepted
Context
=======
The toggle classes WaffleFlag and WaffleSwitch rely on several namespace classes (WaffleNamespace, WaffleSwitchNamespace, and WaffleFlagNamespace). In order to create a WaffleFlag (or WaffleSwitch), you must first create a Namespace object.
Other IDAs have active waffle flags and switches that don't use any namespacing, like this `example switch in ecommerce`_. Once WaffleFlag and WaffleSwitch are extracted to be used in other IDAs (see 0002-waffle-utils-extraction), the required namespace class will make this transition more difficult.
Additionally, the fully qualified waffle name, including the namespace, is required in code annotations and the django admin. Since it needs to be manually reconstructed by the developer, it has lead to copy/paste issues that are also difficult to lint.
Lastly, the namespace classes contain a lot of logic, but in effect, they only are used to ensure the flag name has a prefix like '<NAMESPACE_NAME>.<FLAG_NAME>'.
.. _example switch in ecommerce: https://github.com/edx/ecommerce/blob/e899c78325ac492d0a2b1ea0aab4d5e230262b8f/ecommerce/extensions/dashboard/users/views.py#L21
Decision
========
Change the interface to WaffleFlag and WaffleSwitch to simply take the complete flag name, rather than a Namespace object.
The constructor can assert that the name includes a `.` to help remind people to use some form of prefixed namespace. However, an optional argument with a name like `skip_namespace_assertion=True` could be used to skip this assertion, enabling a simpler transition for existing flags and switches that don't meet this requirement.
Consequences
============
This change will enable WaffleFlag, WaffleSwitch, and all subclasses to have a simpler interface. In addition to a simpler constructor, we will no longer need to differentiate between an instance's namespaced and non-namespaced name.
A possible rollout plan would be to introduce WaffleFlag and WaffleSwitch classes with the new interface when they are added into edx-toggles, and deprecate the old versions in edx-platform. This would enable us to reuse the same class names with a new import, for an iterative rollout.
Although it would be nice to update CourseWaffleFlag to have a similar interface for consistency, it is a lower priority if it is not moving to edx-toggles. See 0003-leave-course-waffle-flag.rst.
This change needs be documented for the next Open edX release.