Skip to content

Commit

Permalink
feat(python): reject registration of custom strategies that don't qua…
Browse files Browse the repository at this point in the history
…ck like ducks
  • Loading branch information
sighphyre committed Nov 11, 2024
1 parent 95e0c4a commit 00db21e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
36 changes: 36 additions & 0 deletions python-engine/tests/test_custom_strategy.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from yggdrasil_engine.custom_strategy import CustomStrategyHandler
import pytest

RAW_STATE = """
{
Expand Down Expand Up @@ -75,3 +76,38 @@ def apply(self, parameters, _context):
results = handler.evaluate_custom_strategies("Feature.A", {})
assert results["customStrategy1"] == True
assert results["customStrategy2"] == False


def test_register_custom_strategy_rejects_strategies_without_apply_method():
class IncompleteStrategy:
pass

handler = CustomStrategyHandler()

with pytest.raises(ValueError):
handler.register_custom_strategies({"custom": IncompleteStrategy()})


def test_register_custom_strategy_rejects_strategies_with_incorrect_number_of_args():
class FirstBearStrategy:
def apply(self, _parameters):
return True

class SecondBearStrategy:
def apply(self, _parameters, _context, _extra):
return True

class ThirdBearStrategy:
def apply(self, _parameters, _context):
return True

handler = CustomStrategyHandler()

with pytest.raises(ValueError):
handler.register_custom_strategies({"custom": FirstBearStrategy()})

with pytest.raises(ValueError):
handler.register_custom_strategies({"custom": SecondBearStrategy()})

## Goldilocks parameters, we expect this not to raise
handler.register_custom_strategies({"custom": ThirdBearStrategy()})
9 changes: 9 additions & 0 deletions python-engine/yggdrasil_engine/custom_strategy.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
from typing import Dict
import inspect

_STANDARD_STRATEGIES = [
"default",
Expand Down Expand Up @@ -41,6 +42,14 @@ def update_strategies(self, features_json: str):
def register_custom_strategies(self, custom_strategies: Dict[str, any]):
for strategy_name, strategy in custom_strategies.items():
if hasattr(strategy, "apply"):
apply_method = strategy.apply
signature = inspect.signature(apply_method)
parameters = list(signature.parameters)
if len(parameters) != 2:
raise ValueError(
f"Custom strategy '{strategy_name}' does not have an apply method "
f"with exactly two parameters. Found {len(parameters)}."
)
self.strategy_implementations[strategy_name] = strategy
else:
raise ValueError(
Expand Down

0 comments on commit 00db21e

Please sign in to comment.