Skip to content

Commit

Permalink
depr: templated hooks
Browse files Browse the repository at this point in the history
Templated hooks we almost completely useless, so we get rid of them.
This allows us to get rid entirely of hook names and hook indexes, which
makes the whole implementation much simpler. Hook removal (with
`clear_all`) is achieved thanks to weak references.
  • Loading branch information
regisb committed Apr 28, 2023
1 parent 8e64ef4 commit 83f21ab
Show file tree
Hide file tree
Showing 21 changed files with 175 additions and 419 deletions.
6 changes: 6 additions & 0 deletions changelog.d/20230412_100608_regis_palm.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,9 @@
- 💥[Improvement] During registration, the honor code and terms of service links are no longer visible by default. For most platforms, these links did not work anyway.
- 💥[Deprecation] Halt support for Python 3.7. The binary release of Tutor is also no longer compatible with macOS 10.
- 💥[Deprecation] Drop support for `docker-compose`, also known as Compose V1. The `docker compose` (no hyphen) plugin must be installed.
- 💥[Refactor] We simplify the hooks API by getting rid of the `ContextTemplate`, `FilterTemplate` and `ActionTemplate` classes. As a consequences, the following changes occur:
- `APP` was previously a ContextTemplate, and is now a dictionary of contexts indexed by name. Developers who implemented this context should replace `Contexts.APP(...)` by `Contexts.app(...)`.
- Removed the `ENV_PATCH` filter, which was for internal use only anyway.
- The `PLUGIN_LOADED` ActionTemplate is now an Action which takes a single argument. (the plugin name)
- 💥[Refactor] We refactored the hooks API further by removing the static hook indexes and the hooks names. As a consequence:
- The syntactic sugar functions from the "filters" and "actions" modules were all removed: `get`, `add*`, `iterate*`, `apply*`, `do*`, etc.
3 changes: 0 additions & 3 deletions docs/reference/api/hooks/actions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ Actions are one of the two types of hooks (the other being :ref:`filters`) that
.. autoclass:: tutor.core.hooks.Action
:members:

.. autoclass:: tutor.core.hooks.ActionTemplate
:members:

.. The following are only to ensure that the docs build without warnings
.. class:: tutor.core.hooks.actions.T
.. class:: tutor.types.Config
3 changes: 0 additions & 3 deletions docs/reference/api/hooks/filters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ Filters are one of the two types of hooks (the other being :ref:`actions`) that
.. autoclass:: tutor.core.hooks.Filter
:members:

.. autoclass:: tutor.core.hooks.FilterTemplate
:members:

.. The following are only to ensure that the docs build without warnings
.. class:: tutor.core.hooks.filters.T1
.. class:: tutor.core.hooks.filters.T2
Expand Down
2 changes: 1 addition & 1 deletion tests/commands/test_config.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import unittest

from tutor import config as tutor_config
from tests.helpers import temporary_root
from tutor import config as tutor_config

from .base import TestCommandMixin

Expand Down
22 changes: 11 additions & 11 deletions tests/core/hooks/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,12 @@ class PluginActionsTests(unittest.TestCase):
def setUp(self) -> None:
self.side_effect_int = 0

def tearDown(self) -> None:
super().tearDown()
actions.clear_all(context="tests")

def run(self, result: t.Any = None) -> t.Any:
with contexts.enter("tests"):
return super().run(result=result)

def test_do(self) -> None:
action: actions.Action[int] = actions.get("test-action")
action: actions.Action[int] = actions.Action()

@action.add()
def _test_action_1(increment: int) -> None:
Expand All @@ -31,29 +27,33 @@ def _test_action_2(increment: int) -> None:
self.assertEqual(3, self.side_effect_int)

def test_priority(self) -> None:
@actions.add("test-action", priority=2)
action: actions.Action[[]] = actions.Action()

@action.add(priority=2)
def _test_action_1() -> None:
self.side_effect_int += 4

@actions.add("test-action", priority=1)
@action.add(priority=1)
def _test_action_2() -> None:
self.side_effect_int = self.side_effect_int // 2

# Action 2 must be performed before action 1
self.side_effect_int = 4
actions.do("test-action")
action.do()
self.assertEqual(6, self.side_effect_int)

def test_equal_priority(self) -> None:
@actions.add("test-action", priority=2)
action: actions.Action[[]] = actions.Action()

@action.add(priority=2)
def _test_action_1() -> None:
self.side_effect_int += 4

@actions.add("test-action", priority=2)
@action.add(priority=2)
def _test_action_2() -> None:
self.side_effect_int = self.side_effect_int // 2

# Action 2 must be performed after action 1
self.side_effect_int = 4
actions.do("test-action")
action.do()
self.assertEqual(4, self.side_effect_int)
44 changes: 22 additions & 22 deletions tests/core/hooks/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,32 @@


class PluginFiltersTests(unittest.TestCase):
def tearDown(self) -> None:
super().tearDown()
filters.clear_all(context="tests")

def run(self, result: t.Any = None) -> t.Any:
with contexts.enter("tests"):
return super().run(result=result)

def test_add(self) -> None:
@filters.add("tests:count-sheeps")
filtre: filters.Filter[int, []] = filters.Filter()

@filtre.add()
def filter1(value: int) -> int:
return value + 1

value = filters.apply("tests:count-sheeps", 0)
value = filtre.apply(0)
self.assertEqual(1, value)

def test_add_items(self) -> None:
@filters.add("tests:add-sheeps")
filtre: filters.Filter[list[int], []] = filters.Filter()

@filtre.add()
def filter1(sheeps: list[int]) -> list[int]:
return sheeps + [0]

filters.add_item("tests:add-sheeps", 1)
filters.add_item("tests:add-sheeps", 2)
filters.add_items("tests:add-sheeps", [3, 4])
filtre.add_item(1)
filtre.add_item(2)
filtre.add_items([3, 4])

sheeps: list[int] = filters.apply("tests:add-sheeps", [])
sheeps: list[int] = filtre.apply([])
self.assertEqual([0, 1, 2, 3, 4], sheeps)

def test_filter_callbacks(self) -> None:
Expand All @@ -42,20 +42,20 @@ def test_filter_callbacks(self) -> None:
self.assertEqual(1, callback.apply(0))

def test_filter_context(self) -> None:
filtre: filters.Filter[list[int], []] = filters.Filter()
with contexts.enter("testcontext"):
filters.add_item("test:sheeps", 1)
filters.add_item("test:sheeps", 2)
filtre.add_item(1)
filtre.add_item(2)

self.assertEqual([1, 2], filters.apply("test:sheeps", []))
self.assertEqual(
[1], filters.apply_from_context("testcontext", "test:sheeps", [])
)
self.assertEqual([1, 2], filtre.apply([]))
self.assertEqual([1], filtre.apply_from_context("testcontext", []))

def test_clear_context(self) -> None:
filtre: filters.Filter[list[int], []] = filters.Filter()
with contexts.enter("testcontext"):
filters.add_item("test:sheeps", 1)
filters.add_item("test:sheeps", 2)
filtre.add_item(1)
filtre.add_item(2)

self.assertEqual([1, 2], filters.apply("test:sheeps", []))
filters.clear("test:sheeps", context="testcontext")
self.assertEqual([2], filters.apply("test:sheeps", []))
self.assertEqual([1, 2], filtre.apply([]))
filtre.clear(context="testcontext")
self.assertEqual([2], filtre.apply([]))
2 changes: 1 addition & 1 deletion tests/test_plugins_v0.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def test_patches(self) -> None:

def test_plugin_without_patches(self) -> None:
plugins_v0.DictPlugin({"name": "plugin1"})
plugins.load("plugin1")
plugins.load_all(["plugin1"])
patches = list(plugins.iter_patches("patch1"))
self.assertEqual([], patches)

Expand Down
2 changes: 1 addition & 1 deletion tutor/bindmount.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from __future__ import annotations

from functools import lru_cache
import os
import re
import typing as t
from functools import lru_cache

from tutor import hooks

Expand Down
3 changes: 2 additions & 1 deletion tutor/commands/compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@

import click

from tutor import bindmount
from tutor import config as tutor_config
from tutor import env as tutor_env
from tutor import bindmount, hooks, utils
from tutor import hooks, utils
from tutor.commands import jobs
from tutor.commands.context import BaseTaskContext
from tutor.core.hooks import Filter # pylint: disable=unused-import
Expand Down
8 changes: 4 additions & 4 deletions tutor/commands/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ def _add_core_init_tasks() -> None:
The context is important, because it allows us to select the init scripts based on
the --limit argument.
"""
with hooks.Contexts.APP("mysql").enter():
with hooks.Contexts.app("mysql").enter():
hooks.Filters.CLI_DO_INIT_TASKS.add_item(
("mysql", env.read_core_template_file("jobs", "init", "mysql.sh"))
)
with hooks.Contexts.APP("lms").enter():
with hooks.Contexts.app("lms").enter():
hooks.Filters.CLI_DO_INIT_TASKS.add_item(
(
"lms",
Expand All @@ -54,7 +54,7 @@ def _add_core_init_tasks() -> None:
hooks.Filters.CLI_DO_INIT_TASKS.add_item(
("lms", env.read_core_template_file("jobs", "init", "lms.sh"))
)
with hooks.Contexts.APP("cms").enter():
with hooks.Contexts.app("cms").enter():
hooks.Filters.CLI_DO_INIT_TASKS.add_item(
("cms", env.read_core_template_file("jobs", "init", "cms.sh"))
)
Expand All @@ -64,7 +64,7 @@ def _add_core_init_tasks() -> None:
@click.option("-l", "--limit", help="Limit initialisation to this service or plugin")
def initialise(limit: t.Optional[str]) -> t.Iterator[tuple[str, str]]:
fmt.echo_info("Initialising all services...")
filter_context = hooks.Contexts.APP(limit).name if limit else None
filter_context = hooks.Contexts.app(limit).name if limit else None

# Deprecated pre-init tasks
for service, path in hooks.Filters.COMMANDS_PRE_INIT.iterate_from_context(
Expand Down
2 changes: 1 addition & 1 deletion tutor/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def _remove_plugin_config_overrides_on_unload(
# Find the configuration entries that were overridden by the plugin and
# remove them from the current config
for key, _value in hooks.Filters.CONFIG_OVERRIDES.iterate_from_context(
hooks.Contexts.APP(plugin).name
hooks.Contexts.app(plugin).name
):
value = config.pop(key, None)
value = env.render_unknown(config, value)
Expand Down
12 changes: 5 additions & 7 deletions tutor/core/hooks/__init__.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import typing as t

from .actions import Action, ActionTemplate
from .actions import clear_all as _clear_all_actions
from .contexts import Context, ContextTemplate
from .filters import Filter, FilterTemplate
from .filters import clear_all as _clear_all_filters
from .actions import Action
from .contexts import Context
from .filters import Filter


def clear_all(context: t.Optional[str] = None) -> None:
"""
Clear both actions and filters.
"""
_clear_all_actions(context=context)
_clear_all_filters(context=context)
Action.clear_all(context=context)
Filter.clear_all(context=context)
Loading

0 comments on commit 83f21ab

Please sign in to comment.