Skip to content

Commit

Permalink
Merge branch 'main' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
Kedro committed Jan 6, 2025
2 parents aa0aad3 + 5721eda commit 181ad47
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Added validation to ensure dataset versions consistency across catalog.
* Fixed a bug in project creation when using a custom starter template offline.
* Added `node` import to the pipeline template.
* Safeguard hooks when user incorrectly registers a hook class in settings.py.

## Breaking changes to the API
## Documentation changes
Expand Down
6 changes: 6 additions & 0 deletions kedro/framework/hooks/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import logging
from collections.abc import Iterable
from inspect import isclass
from typing import Any

from pluggy import PluginManager
Expand Down Expand Up @@ -48,6 +49,11 @@ def _register_hooks(hook_manager: PluginManager, hooks: Iterable[Any]) -> None:
# case hooks have already been registered, so we perform a simple check
# here to avoid an error being raised and break user's workflow.
if not hook_manager.is_registered(hooks_collection):
if isclass(hooks_collection):
raise TypeError(
"KedroSession expects hooks to be registered as instances. "
"Have you forgotten the `()` when registering a hook class ?"
)
hook_manager.register(hooks_collection)


Expand Down
31 changes: 30 additions & 1 deletion tests/framework/hooks/test_manager.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import pytest
from pluggy import PluginManager

from kedro.framework.hooks.manager import _create_hook_manager, _NullPluginManager
from kedro.framework.hooks.manager import (
_create_hook_manager,
_NullPluginManager,
_register_hooks,
)
from kedro.framework.hooks.specs import (
DataCatalogSpecs,
DatasetSpecs,
Expand All @@ -10,6 +15,10 @@
)


class ExampleHook:
pass


@pytest.mark.parametrize(
"hook_specs,hook_name,hook_params",
[
Expand Down Expand Up @@ -73,3 +82,23 @@ def test_null_plugin_manager_returns_none_when_called():
assert (
plugin_manager.hook.before_dataset_saved(dataset_name="mock", data=[]) is None
)


@pytest.mark.parametrize(
"hooks, should_raise",
[
([ExampleHook], True),
([ExampleHook()], False),
],
)
def test_register_hooks(hooks, should_raise):
mock_hook_manager = PluginManager("test_project")

if should_raise:
with pytest.raises(
TypeError, match="KedroSession expects hooks to be registered as instances"
):
_register_hooks(mock_hook_manager, hooks)
else:
_register_hooks(mock_hook_manager, hooks)
assert mock_hook_manager.is_registered(hooks[0])

0 comments on commit 181ad47

Please sign in to comment.