From e39435d4447bc8370798a92a89b97a88a2e02ebb Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 7 Aug 2024 15:31:04 -0400 Subject: [PATCH 01/16] feat: add ability to override XBlock with xblock.v1.overrides entry_point --- xblock/plugin.py | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index 3f1574c4b..fb0888e08 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -41,6 +41,37 @@ def default_select(identifier, all_entry_points): # pylint: disable=inconsisten raise AmbiguousPluginError(all_entry_points) +def select_with_overrides(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements + """ + Return overridden entry point, if present. Otherwise returns entry point. + Raise an exception when we have ambiguous entry points. + """ + + # Split entry points into overrides and non-overrides + overrides = [] + block_entry_points = [] + + for block_entry_point in all_entry_points: + if "overrides" in block_entry_point.group: + overrides.append(block_entry_point) + else: + block_entry_points.append(block_entry_point) + + # Overrides get priority + if len(overrides) == 1: + return overrides[0] + if len(overrides) > 1: + raise AmbiguousPluginError(all_entry_points) + + # ... then default behavior + if len(block_entry_points) == 0: + raise PluginMissingError(identifier) + if len(block_entry_points) == 1: + return all_entry_points[0] + elif len(block_entry_points) > 1: + raise AmbiguousPluginError(all_entry_points) + + class Plugin: """Base class for a system that uses entry_points to load plugins. @@ -98,9 +129,13 @@ def select(identifier, all_entry_points): if key not in PLUGIN_CACHE: if select is None: - select = default_select + select = select_with_overrides + + all_entry_points = [ + *importlib.metadata.entry_points(group=f'{cls.entry_point}.overrides', name=identifier), + *importlib.metadata.entry_points(group=cls.entry_point, name=identifier) + ] - all_entry_points = list(importlib.metadata.entry_points(group=cls.entry_point, name=identifier)) for extra_identifier, extra_entry_point in iter(cls.extra_entry_points): if identifier == extra_identifier: all_entry_points.append(extra_entry_point) @@ -146,7 +181,7 @@ def load_classes(cls, fail_silently=True): raise @classmethod - def register_temp_plugin(cls, class_, identifier=None, dist='xblock'): + def register_temp_plugin(cls, class_, identifier=None, dist='xblock', group='xblock.v1'): """Decorate a function to run with a temporary plugin available. Use it like this in tests:: @@ -164,6 +199,7 @@ def test_the_thing(): entry_point = Mock( dist=Mock(key=dist), load=Mock(return_value=class_), + group=group ) entry_point.name = identifier From 95f538559092e5299548e1c9c7ed50da98e170f9 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 7 Aug 2024 15:31:16 -0400 Subject: [PATCH 02/16] test: xblock overrides --- xblock/test/test_plugin.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/xblock/test/test_plugin.py b/xblock/test/test_plugin.py index c2832cdb1..7bad5721e 100644 --- a/xblock/test/test_plugin.py +++ b/xblock/test/test_plugin.py @@ -21,6 +21,10 @@ class UnambiguousBlock(XBlock): """A dummy class to find as a plugin.""" +class OverriddenBlock(XBlock): + """A dummy class to find as a plugin.""" + + @XBlock.register_temp_plugin(AmbiguousBlock1, "bad_block") @XBlock.register_temp_plugin(AmbiguousBlock2, "bad_block") @XBlock.register_temp_plugin(UnambiguousBlock, "good_block") @@ -52,6 +56,35 @@ def boom(identifier, entry_points): XBlock.load_class("bad_block", select=boom) +@XBlock.register_temp_plugin(OverriddenBlock, "overridden_block", group='xblock.v1.overrides') +@XBlock.register_temp_plugin(AmbiguousBlock1, "overridden_block") +@XBlock.register_temp_plugin(AmbiguousBlock2, "overridden_block") +@XBlock.register_temp_plugin(UnambiguousBlock, "good_block") +def test_plugin_override(): + # We can load ok blocks even if there are bad blocks. + cls = XBlock.load_class("good_block") + assert cls is UnambiguousBlock + + # Trying to load a block that is overridden returns the correct override + override = XBlock.load_class("overridden_block") + assert override is OverriddenBlock + + +@XBlock.register_temp_plugin(AmbiguousBlock1, "overridden_block", group='xblock.v1.overrides') +@XBlock.register_temp_plugin(AmbiguousBlock2, "overridden_block", group='xblock.v1.overrides') +@XBlock.register_temp_plugin(OverriddenBlock, "overridden_block") +def test_plugin_override_ambiguous(): + + # Trying to load a block that is overridden, but ambigous, errors. + expected_msg = ( + "Ambiguous entry points for overridden_block: " + "xblock.test.test_plugin.AmbiguousBlock1, " + "xblock.test.test_plugin.AmbiguousBlock2" + ) + with pytest.raises(AmbiguousPluginError, match=expected_msg): + XBlock.load_class("overridden_block") + + def test_nosuch_plugin(): # We can provide a default class to return for missing plugins. cls = XBlock.load_class("nosuch_block", default=UnambiguousBlock) From 5ea811845c8bad5b30db1c38217c65573832febb Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 7 Aug 2024 15:31:36 -0400 Subject: [PATCH 03/16] chore: bump version to 5.1.0 --- xblock/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xblock/__init__.py b/xblock/__init__.py index 9ba6b90d0..cd74464e7 100644 --- a/xblock/__init__.py +++ b/xblock/__init__.py @@ -2,4 +2,4 @@ XBlock Courseware Components """ -__version__ = '5.0.0' +__version__ = '5.1.0' From 3e08b42f4fb55efd14236273f990eb9f3adbba03 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 7 Aug 2024 15:31:43 -0400 Subject: [PATCH 04/16] docs: add changelog entry --- CHANGELOG.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6626a4786..7242b2fae 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,12 @@ Change history for XBlock Unreleased ---------- +5.1.0 - 2024-08-07 +------------------ + +* added ability to override an XBlock with the 'xblock.{version}.overrides' entry point. + + 5.0.0 - 2024-05-30 ------------------ From 72a35fd2e3ae1611f97e7cbd41b65cc9a7a843a6 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Thu, 8 Aug 2024 11:13:23 -0400 Subject: [PATCH 05/16] refactor: move override behavior into default select --- xblock/plugin.py | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index fb0888e08..17c324223 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -30,20 +30,9 @@ def __init__(self, all_entry_points): def default_select(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements """ - Raise an exception when we have ambiguous entry points. - """ - - if len(all_entry_points) == 0: - raise PluginMissingError(identifier) - if len(all_entry_points) == 1: - return all_entry_points[0] - elif len(all_entry_points) > 1: - raise AmbiguousPluginError(all_entry_points) + Select the appropriate entry point for a given block. + Honors the ability to override and take preference over the default entry point. - -def select_with_overrides(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements - """ - Return overridden entry point, if present. Otherwise returns entry point. Raise an exception when we have ambiguous entry points. """ @@ -60,13 +49,13 @@ def select_with_overrides(identifier, all_entry_points): # pylint: disable=inco # Overrides get priority if len(overrides) == 1: return overrides[0] - if len(overrides) > 1: + elif len(overrides) > 1: raise AmbiguousPluginError(all_entry_points) # ... then default behavior - if len(block_entry_points) == 0: + elif len(block_entry_points) == 0: raise PluginMissingError(identifier) - if len(block_entry_points) == 1: + elif len(block_entry_points) == 1: return all_entry_points[0] elif len(block_entry_points) > 1: raise AmbiguousPluginError(all_entry_points) @@ -129,7 +118,7 @@ def select(identifier, all_entry_points): if key not in PLUGIN_CACHE: if select is None: - select = select_with_overrides + select = default_select all_entry_points = [ *importlib.metadata.entry_points(group=f'{cls.entry_point}.overrides', name=identifier), From a75d479204d16312f6627611e62191b0353e0c22 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Thu, 8 Aug 2024 11:13:38 -0400 Subject: [PATCH 06/16] refactor: update logic for checking override --- xblock/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index 17c324223..eccaa391a 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -41,7 +41,7 @@ def default_select(identifier, all_entry_points): # pylint: disable=inconsisten block_entry_points = [] for block_entry_point in all_entry_points: - if "overrides" in block_entry_point.group: + if block_entry_point.group.endswith('overrides'): overrides.append(block_entry_point) else: block_entry_points.append(block_entry_point) From d68b00d05ab8c90094ec392b83e93871d04f34e0 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Thu, 8 Aug 2024 11:18:43 -0400 Subject: [PATCH 07/16] refactor: split override / default select, use default select in override --- xblock/plugin.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index eccaa391a..b5b1d20c9 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -30,9 +30,20 @@ def __init__(self, all_entry_points): def default_select(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements """ - Select the appropriate entry point for a given block. - Honors the ability to override and take preference over the default entry point. + Raise an exception when we have ambiguous entry points. + """ + + if len(all_entry_points) == 0: + raise PluginMissingError(identifier) + if len(all_entry_points) == 1: + return all_entry_points[0] + elif len(all_entry_points) > 1: + raise AmbiguousPluginError(all_entry_points) + +def select_with_overrides(identifier, all_entry_points): + """ + Honors the ability for an XBlock to override the default entry point. Raise an exception when we have ambiguous entry points. """ @@ -53,12 +64,7 @@ def default_select(identifier, all_entry_points): # pylint: disable=inconsisten raise AmbiguousPluginError(all_entry_points) # ... then default behavior - elif len(block_entry_points) == 0: - raise PluginMissingError(identifier) - elif len(block_entry_points) == 1: - return all_entry_points[0] - elif len(block_entry_points) > 1: - raise AmbiguousPluginError(all_entry_points) + return default_select(identifier, block_entry_points) class Plugin: @@ -118,7 +124,7 @@ def select(identifier, all_entry_points): if key not in PLUGIN_CACHE: if select is None: - select = default_select + select = select_with_overrides all_entry_points = [ *importlib.metadata.entry_points(group=f'{cls.entry_point}.overrides', name=identifier), From 35d242345c84f2fcb5b3d976a1a65fcdf34547fa Mon Sep 17 00:00:00 2001 From: Nathan Sprenkle Date: Tue, 13 Aug 2024 11:22:30 -0400 Subject: [PATCH 08/16] docs: update changelog entry Co-authored-by: Kyle McCormick --- CHANGELOG.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7242b2fae..6a5eff3a9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,7 +8,8 @@ Unreleased 5.1.0 - 2024-08-07 ------------------ -* added ability to override an XBlock with the 'xblock.{version}.overrides' entry point. +* added ability to override an XBlock with the 'xblock.v1.overrides' entry point +* added ability to override an XBlock Aside with the 'xblock_asides.v1.overrides' entry point 5.0.0 - 2024-05-30 From 57215862b29234378028fc9c17d8cc8dd72ae632 Mon Sep 17 00:00:00 2001 From: Nathan Sprenkle Date: Tue, 13 Aug 2024 11:23:07 -0400 Subject: [PATCH 09/16] feat: update override checking to include leading period Co-authored-by: Kyle McCormick --- xblock/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index b5b1d20c9..faac30f98 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -52,7 +52,7 @@ def select_with_overrides(identifier, all_entry_points): block_entry_points = [] for block_entry_point in all_entry_points: - if block_entry_point.group.endswith('overrides'): + if block_entry_point.group.endswith('.overrides'): overrides.append(block_entry_point) else: block_entry_points.append(block_entry_point) From 607e7dca044389817c2da1fe78d49dd20161c065 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 14 Aug 2024 13:30:27 -0400 Subject: [PATCH 10/16] refactor: make default select use overrides, raise if overridden block not found --- xblock/plugin.py | 15 ++++++++------- xblock/test/test_plugin.py | 8 +------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index faac30f98..0ed03659b 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -28,7 +28,7 @@ def __init__(self, all_entry_points): super().__init__(msg) -def default_select(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements +def _default_select_no_override(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements """ Raise an exception when we have ambiguous entry points. """ @@ -41,7 +41,7 @@ def default_select(identifier, all_entry_points): # pylint: disable=inconsisten raise AmbiguousPluginError(all_entry_points) -def select_with_overrides(identifier, all_entry_points): +def default_select(identifier, all_entry_points): """ Honors the ability for an XBlock to override the default entry point. Raise an exception when we have ambiguous entry points. @@ -57,14 +57,15 @@ def select_with_overrides(identifier, all_entry_points): else: block_entry_points.append(block_entry_point) - # Overrides get priority + # Get the default entry point + default_plugin = _default_select_no_override(identifier, block_entry_points) + + # If we have an unambiguous override, that gets priority. Otherwise, return default. if len(overrides) == 1: return overrides[0] elif len(overrides) > 1: raise AmbiguousPluginError(all_entry_points) - - # ... then default behavior - return default_select(identifier, block_entry_points) + return default_plugin class Plugin: @@ -124,7 +125,7 @@ def select(identifier, all_entry_points): if key not in PLUGIN_CACHE: if select is None: - select = select_with_overrides + select = default_select all_entry_points = [ *importlib.metadata.entry_points(group=f'{cls.entry_point}.overrides', name=identifier), diff --git a/xblock/test/test_plugin.py b/xblock/test/test_plugin.py index 7bad5721e..1bd97c11b 100644 --- a/xblock/test/test_plugin.py +++ b/xblock/test/test_plugin.py @@ -57,14 +57,8 @@ def boom(identifier, entry_points): @XBlock.register_temp_plugin(OverriddenBlock, "overridden_block", group='xblock.v1.overrides') -@XBlock.register_temp_plugin(AmbiguousBlock1, "overridden_block") -@XBlock.register_temp_plugin(AmbiguousBlock2, "overridden_block") -@XBlock.register_temp_plugin(UnambiguousBlock, "good_block") +@XBlock.register_temp_plugin(UnambiguousBlock, "overridden_block") def test_plugin_override(): - # We can load ok blocks even if there are bad blocks. - cls = XBlock.load_class("good_block") - assert cls is UnambiguousBlock - # Trying to load a block that is overridden returns the correct override override = XBlock.load_class("overridden_block") assert override is OverriddenBlock From 3911cef4fc47244c9b6a9d00ae41dd16461fdeee Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 14 Aug 2024 13:30:39 -0400 Subject: [PATCH 11/16] docs: update docstrings --- xblock/plugin.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index 0ed03659b..ad7b8d502 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -30,7 +30,11 @@ def __init__(self, all_entry_points): def _default_select_no_override(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements """ - Raise an exception when we have ambiguous entry points. + Selects plugin for the given identifier, raising on error: + + Raises: + - PluginMissingError when we don't have an entry point. + - AmbiguousPluginError when we have ambiguous entry points. """ if len(all_entry_points) == 0: @@ -43,8 +47,12 @@ def _default_select_no_override(identifier, all_entry_points): # pylint: disabl def default_select(identifier, all_entry_points): """ - Honors the ability for an XBlock to override the default entry point. - Raise an exception when we have ambiguous entry points. + Selects plugin for the given identifier with the ability for a Plugin to override + the default entry point. + + Raises: + - PluginMissingError when we don't have an entry point or entry point to override. + - AmbiguousPluginError when we have ambiguous entry points. """ # Split entry points into overrides and non-overrides @@ -102,12 +110,20 @@ def _load_class_entry_point(cls, entry_point): def load_class(cls, identifier, default=None, select=None): """Load a single class specified by identifier. - If `identifier` specifies more than a single class, and `select` is not None, - then call `select` on the list of entry_points. Otherwise, choose - the first one and log a warning. + By default, this returns the class mapped to `identifier` from entry_points + matching `{cls.entry_points}.overrides` or `{cls.entry_points}`, in that order. + + If multiple classes are found for either `{cls.entry_points}.overrides` or + `{cls.entry_points}`, it will raise an `AmbiguousPluginError`. + + If no classes are found for `{cls.entry_points}`, it will raise a `PluginMissingError`. + + Args: + - identifier: The class to match on. - If `default` is provided, return it if no entry_point matching - `identifier` is found. Otherwise, will raise a PluginMissingError + Kwargs: + - default: A class to return if no entry_point matching `identifier` is found. + - select: A function to override our default_select functionality. If `select` is provided, it should be a callable of the form:: From b86898eb16dcccd09b9bd75b11f4b8bb912e53d5 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 14 Aug 2024 13:53:00 -0400 Subject: [PATCH 12/16] docs: add tutorial for overriding XBlock --- docs/xblock-tutorial/edx_platform/index.rst | 1 + .../edx_platform/overrides.rst | 75 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 docs/xblock-tutorial/edx_platform/overrides.rst diff --git a/docs/xblock-tutorial/edx_platform/index.rst b/docs/xblock-tutorial/edx_platform/index.rst index c57b5eb59..5b1eba954 100644 --- a/docs/xblock-tutorial/edx_platform/index.rst +++ b/docs/xblock-tutorial/edx_platform/index.rst @@ -11,3 +11,4 @@ XBlocks and the edX Platform edx_lms devstack edx + overrides diff --git a/docs/xblock-tutorial/edx_platform/overrides.rst b/docs/xblock-tutorial/edx_platform/overrides.rst new file mode 100644 index 000000000..2ada4d272 --- /dev/null +++ b/docs/xblock-tutorial/edx_platform/overrides.rst @@ -0,0 +1,75 @@ +.. _Replace a Preinstalled XBlock With a Custom Implementation: + +########################################################## +Replace a Preinstalled XBlock With a Custom Implementation +########################################################## + +In XBlock ``v5.1.0``, the ability was introduced to override an XBlock with a custom +implementation. + +This can be done by: + +1. Creating an XBlock in a new or existing Django Plugin, installed into ``edx-platform``. + +2. Adding the ``xblock.v1.overrides`` entry point in ``setup.py``, pointing to your + custom XBlock. + +This works with updated logic in ``load_class``'s ``default_select``, which gives +load priority to a class with the ``.overrides`` suffix. + +This can be disabled by providing a different ``select`` kwarg to ``load_class`` which +ignores or otherwise changes override logic. + +******* +Example +******* + +Imagine there is an XBlock installed ``edx-platform``: + +.. code:: python + + # edx-platform/xblocks/twitch_plays_pokemon.py + class TwitchPlaysPokemonBlock(XBlock): + ... + + # edx-platform/setup.py + setup( + # ... + + entry_points={ + "xblock.v1": [ + "tpp = xblocks.twitch_plays_pokemon::TwitchPlaysPokemonBlock" + # ... + ] + } + ) + +If you then create your own Plugin App with a custom version of that XBlock... + +.. code:: python + + # your_plugin/xblocks/twitch_plays_pokemon.py + class YourTwitchPlaysPokemonBlock(XBlock): + ... + + # your_plugin/setup.py + setup( + # ... + entry_points={ + "xblock.v1.overrides": [ + "tpp = your_plugin.twitch_plays_pokemon::YourTwitchPlaysPokemonBlock" + # ... + ], + } + ) + +And install that app as a Django plugin, then your block should be loaded instead of the +existing implementation. + +.. note:: + + The ``load_class`` code will throw an error in the following cases: + + 1. There are multiple classes attempting to override one XBlock implementation. + + 2. There is an override provided where an existing XBlock implementation is not found. From f9b33d47df4f9ed97f330dbece4f9000a839e76f Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Thu, 15 Aug 2024 12:25:58 -0400 Subject: [PATCH 13/16] docs: update documentation --- .../edx_platform/overrides.rst | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/xblock-tutorial/edx_platform/overrides.rst b/docs/xblock-tutorial/edx_platform/overrides.rst index 2ada4d272..090e8a04f 100644 --- a/docs/xblock-tutorial/edx_platform/overrides.rst +++ b/docs/xblock-tutorial/edx_platform/overrides.rst @@ -9,7 +9,7 @@ implementation. This can be done by: -1. Creating an XBlock in a new or existing Django Plugin, installed into ``edx-platform``. +1. Creating an XBlock in a new or existing package installed into ``edx-platform``. 2. Adding the ``xblock.v1.overrides`` entry point in ``setup.py``, pointing to your custom XBlock. @@ -28,8 +28,8 @@ Imagine there is an XBlock installed ``edx-platform``: .. code:: python - # edx-platform/xblocks/twitch_plays_pokemon.py - class TwitchPlaysPokemonBlock(XBlock): + # edx-platform/xblocks/video_block.py + class VideoBlock(XBlock): ... # edx-platform/setup.py @@ -38,18 +38,18 @@ Imagine there is an XBlock installed ``edx-platform``: entry_points={ "xblock.v1": [ - "tpp = xblocks.twitch_plays_pokemon::TwitchPlaysPokemonBlock" + "video = xblocks.video_block::VideoBlock" # ... ] } ) -If you then create your own Plugin App with a custom version of that XBlock... +If you then create your own Python package with a custom version of that XBlock... .. code:: python - # your_plugin/xblocks/twitch_plays_pokemon.py - class YourTwitchPlaysPokemonBlock(XBlock): + # your_plugin/xblocks/video_block.py + class YourVideoBlock(XBlock): ... # your_plugin/setup.py @@ -57,14 +57,14 @@ If you then create your own Plugin App with a custom version of that XBlock... # ... entry_points={ "xblock.v1.overrides": [ - "tpp = your_plugin.twitch_plays_pokemon::YourTwitchPlaysPokemonBlock" + "tpp = your_plugin.xblocks.video_block::YourVideoBlock" # ... ], } ) -And install that app as a Django plugin, then your block should be loaded instead of the -existing implementation. +And install that package into your virtual environment, then your block should be +loaded instead of the existing implementation. .. note:: From 2a8ade34a1e75e20abdf7a0c9e338f0c667adbd0 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Thu, 15 Aug 2024 14:29:21 -0400 Subject: [PATCH 14/16] test: add test for override without base block --- xblock/test/test_plugin.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/xblock/test/test_plugin.py b/xblock/test/test_plugin.py index 1bd97c11b..681d5c634 100644 --- a/xblock/test/test_plugin.py +++ b/xblock/test/test_plugin.py @@ -64,6 +64,13 @@ def test_plugin_override(): assert override is OverriddenBlock +@XBlock.register_temp_plugin(OverriddenBlock, "overridden_block", group='xblock.v1.overrides') +def test_plugin_override_missing_original(): + # Trying to override a block that has no original block should raise an error + with pytest.raises(PluginMissingError, match="overridden_block"): + XBlock.load_class("overridden_block") + + @XBlock.register_temp_plugin(AmbiguousBlock1, "overridden_block", group='xblock.v1.overrides') @XBlock.register_temp_plugin(AmbiguousBlock2, "overridden_block", group='xblock.v1.overrides') @XBlock.register_temp_plugin(OverriddenBlock, "overridden_block") From f623861fcf9caee79b2f37276077221640cedd01 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Fri, 16 Aug 2024 11:03:14 -0400 Subject: [PATCH 15/16] docs: update documentation --- docs/xblock-tutorial/edx_platform/overrides.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/xblock-tutorial/edx_platform/overrides.rst b/docs/xblock-tutorial/edx_platform/overrides.rst index 090e8a04f..aa3eaf2de 100644 --- a/docs/xblock-tutorial/edx_platform/overrides.rst +++ b/docs/xblock-tutorial/edx_platform/overrides.rst @@ -57,7 +57,7 @@ If you then create your own Python package with a custom version of that XBlock. # ... entry_points={ "xblock.v1.overrides": [ - "tpp = your_plugin.xblocks.video_block::YourVideoBlock" + "video = your_plugin.xblocks.video_block::YourVideoBlock" # ... ], } From b9ac9099e98188e2241fc4914c37c055563d2e48 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Fri, 16 Aug 2024 13:56:00 -0400 Subject: [PATCH 16/16] feat: raise AmbiguousPluginOverrideError for ambibuous plugin overrides --- xblock/plugin.py | 6 +++++- xblock/test/test_plugin.py | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/xblock/plugin.py b/xblock/plugin.py index ad7b8d502..6b499946c 100644 --- a/xblock/plugin.py +++ b/xblock/plugin.py @@ -28,6 +28,10 @@ def __init__(self, all_entry_points): super().__init__(msg) +class AmbiguousPluginOverrideError(AmbiguousPluginError): + """Raised when a class name produces more than one override for an entry_point.""" + + def _default_select_no_override(identifier, all_entry_points): # pylint: disable=inconsistent-return-statements """ Selects plugin for the given identifier, raising on error: @@ -72,7 +76,7 @@ def default_select(identifier, all_entry_points): if len(overrides) == 1: return overrides[0] elif len(overrides) > 1: - raise AmbiguousPluginError(all_entry_points) + raise AmbiguousPluginOverrideError(overrides) return default_plugin diff --git a/xblock/test/test_plugin.py b/xblock/test/test_plugin.py index 681d5c634..08049fa04 100644 --- a/xblock/test/test_plugin.py +++ b/xblock/test/test_plugin.py @@ -6,7 +6,7 @@ from xblock.core import XBlock from xblock import plugin -from xblock.plugin import AmbiguousPluginError, PluginMissingError +from xblock.plugin import AmbiguousPluginError, AmbiguousPluginOverrideError, PluginMissingError class AmbiguousBlock1(XBlock): @@ -82,7 +82,7 @@ def test_plugin_override_ambiguous(): "xblock.test.test_plugin.AmbiguousBlock1, " "xblock.test.test_plugin.AmbiguousBlock2" ) - with pytest.raises(AmbiguousPluginError, match=expected_msg): + with pytest.raises(AmbiguousPluginOverrideError, match=expected_msg): XBlock.load_class("overridden_block")