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

Add function to unregister a previously registered hook. #12100

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions kolibri/plugins/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,28 @@ class and all parent abstract classes - can only be called on an abstract
return subclass


def unregister_hook(subclass):
"""
This method must be used as a decorator to unregister a hook and prevent it from being
registered against the abstract hook classes it inherits from.
"""
if not any(
hasattr(base, "_registered_hooks")
and base.abstract
and issubclass(base, KolibriHook)
for base in subclass.__bases__
):
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here regarding consolidation. Seems this validation logic is pretty much the same as register_hook

raise TypeError(
"unregister_hook decorator used on a class that does not inherit from any abstract KolibriHook subclasses"
)
if not subclass.__module__.endswith("kolibri_plugin"):
raise RuntimeError(
"unregister_hook decorator invoked outside of a kolibri_plugin.py module - this hook will not be initialized"
)
subclass.remove_hook_from_registries()
subclass._registered = False


class KolibriHookMeta(SingletonMeta):
"""
We use a metaclass to define class level properties and methods in a simple way.
Expand Down Expand Up @@ -315,6 +337,37 @@ def add_hook_to_class_registry(cls, hook):
"{} added to registry for defined hook: {}".format(hook.unique_id, cls)
)

def remove_hook_from_registries(cls):
"""
Remove a concrete hook class from all relevant abstract hook registries.
"""
if not cls.abstract and cls._registered:
hook = cls()
for parent in cls.__mro__:
if (
isabstract(parent)
and issubclass(parent, KolibriHook)
and parent is not KolibriHook
and hasattr(parent, "_registered_hooks")
):
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker: some of this logic seems like it's getting a bit repetitive. There are subtle differences, but bulk of it seems like it could be consolidated. Every time I look at this area of the codebase, it takes me a while to carefully understand all of the conditions. Some consolidation can tuck that a way into a well commented chunk of code, so each usage can focus on its goal and what's different

parent.remove_hook_from_class_registry(hook)

def remove_hook_from_class_registry(cls, hook):
"""
Remove a concrete hook instance from the hook registry on this abstract hook
"""
if not cls.abstract:
raise TypeError(
"remove_hook_from_registry method used on a non-abstract hook"
)
if hook.unique_id in cls._registered_hooks:
del cls._registered_hooks[hook.unique_id]
logger.debug(
"{} removed from registry for defined hook: {}".format(
hook.unique_id, cls
)
)

def get_hook(cls, unique_id):
"""
Fetch a registered hook instance by its unique_id
Expand Down
Loading