Skip to content

Yet another refactor #33

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Feb 7, 2022
Merged

Yet another refactor #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3dc3ffd
draft
chadell Jan 20, 2022
afec8b5
proposal
chadell Feb 3, 2022
a656ada
fix tests
chadell Feb 7, 2022
dd7c6a3
fix tolerance tests
lvrfrc87 Feb 7, 2022
2c06b18
fix build
chadell Feb 7, 2022
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
142 changes: 86 additions & 56 deletions netcompare/check_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from typing import Mapping, Tuple, List, Dict, Any, Union
import jmespath


from .utils.jmespath_parsers import (
jmespath_value_parser,
jmespath_refkey_parser,
Expand All @@ -13,29 +14,27 @@
from .utils.data_normalization import exclude_filter, flatten_list
from .evaluators import diff_generator, parameter_evaluator, regex_evaluator

# pylint: disable=arguments-differ


class CheckType:
Copy link
Collaborator

@pszulczewski pszulczewski Feb 3, 2022

Choose a reason for hiding this comment

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

It makes me think that using abstract class could simplify it and still enforce common interface.

from abc import ABC, abstractmethod


class CheckType(ABC):

    @abstractmethod
    def validate(self, **kwargs):
        # Validate logic
        pass

    @abstractmethod
    def evaluate(self, data, **kwargs):
        self.validate(**kwargs)
        # Evaluate logic
        pass


class MyCheck(CheckType):

    def validate(self, **kwargs):
        # check-type logic
        return

    def evaluate(self, data, **kwargs):
        self.validate(**kwargs)
        # check-type logic
        return

Definitely not something to do now, but just wanted to share this as an idea. IMO cleaner and less boilerplate code.

"""Check Type Class."""

def __init__(self, *args):
"""Check Type init method."""

@staticmethod
def init(*args):
def init(check_type):
"""Factory pattern to get the appropriate CheckType implementation.

Args:
*args: Variable length argument list.
check_type: String to define the type of check.
"""
check_type = args[0]
if check_type == "exact_match":
return ExactMatchType(*args)
return ExactMatchType()
if check_type == "tolerance":
return ToleranceType(*args)
return ToleranceType()
if check_type == "parameter_match":
return ParameterMatchType(*args)
return ParameterMatchType()
if check_type == "regex":
return RegexType(*args)
return RegexType()

raise NotImplementedError

Expand Down Expand Up @@ -89,102 +88,133 @@ def get_value(output: Union[Mapping, List], path: str, exclude: List = None) ->

return values

def evaluate(self, reference_value: Any, value_to_compare: Any) -> Tuple[Dict, bool]:
def evaluate(self, value_to_compare: Any, **kwargs) -> Tuple[Dict, bool]:
"""Return the result of the evaluation and a boolean True if it passes it or False otherwise.

This method is the one that each CheckType has to implement.

Args:
reference_value: Can be any structured data or just a simple value.
value_to_compare: Similar value as above to perform comparison.
Copy link
Collaborator

@pszulczewski pszulczewski Feb 3, 2022

Choose a reason for hiding this comment

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

Just remember about updating docstrings, or get someone to do that. I can volunteer.


Returns:
Copy link
Collaborator

@pszulczewski pszulczewski Feb 3, 2022

Choose a reason for hiding this comment

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

What do you think about adding:
self.validator_class.validate(**kwargs) in this abstract method? This would show the proper usage.

tuple: Dictionary representing check result, bool indicating if differences are found.
"""
# This method should call before any other logic the validation of the arguments
# self.validate(**kwargs)
raise NotImplementedError

@staticmethod
def validate(**kwargs):
"""Method to validate arguments that raises proper exceptions."""
raise NotImplementedError


class ExactMatchType(CheckType):
"""Exact Match class docstring."""

def evaluate(self, reference_value: Any, value_to_compare: Any) -> Tuple[Dict, bool]:
@staticmethod
def validate(**kwargs):
"""Method to validate arguments."""
# reference_data = getattr(kwargs, "reference_data")

def evaluate(self, value_to_compare: Any, reference_data: Any) -> Tuple[Dict, bool]:
"""Returns the difference between values and the boolean."""
evaluation_result = diff_generator(reference_value, value_to_compare)
self.validate(reference_data=reference_data)
evaluation_result = diff_generator(reference_data, value_to_compare)
return evaluation_result, not evaluation_result


class ToleranceType(CheckType):
"""Tolerance class docstring."""

def __init__(self, *args):
"""Tolerance init method."""
super().__init__()

try:
tolerance = float(args[1])
except IndexError as error:
raise IndexError(f"Tolerance parameter must be defined as float at index 1. You have: {args}") from error
except ValueError as error:
raise ValueError(f"Argument must be convertible to float. You have: {args[1]}") from error

self.tolerance_factor = tolerance / 100

def evaluate(self, reference_value: Mapping, value_to_compare: Mapping) -> Tuple[Dict, bool]:
@staticmethod
def validate(**kwargs):
"""Method to validate arguments."""
# reference_data = getattr(kwargs, "reference_data")
tolerance = kwargs.get("tolerance")
if not tolerance:
raise ValueError("Tolerance argument is mandatory for Tolerance Check Type.")
if not isinstance(tolerance, int):
raise ValueError(f"Tolerance argument must be an integer, and it's {type(tolerance)}.")

def evaluate(self, value_to_compare: Any, reference_data: Any, tolerance: int) -> Tuple[Dict, bool]:
"""Returns the difference between values and the boolean. Overwrites method in base class."""
diff = diff_generator(reference_value, value_to_compare)
self._remove_within_tolerance(diff)
self.validate(reference_data=reference_data, tolerance=tolerance)
diff = diff_generator(reference_data, value_to_compare)
self._remove_within_tolerance(diff, tolerance)
return diff, not diff

def _remove_within_tolerance(self, diff: Dict) -> None:
def _remove_within_tolerance(self, diff: Dict, tolerance: int) -> None:
"""Recursively look into diff and apply tolerance check, remove reported difference when within tolerance."""

def _within_tolerance(*, old_value: float, new_value: float) -> bool:
"""Return True if new value is within the tolerance range of the previous value."""
max_diff = old_value * self.tolerance_factor
tolerance_factor = tolerance / 100
Copy link
Collaborator

@lvrfrc87 lvrfrc87 Feb 7, 2022

Choose a reason for hiding this comment

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

@chadell this line was missing.

max_diff = old_value * tolerance_factor
return (old_value - max_diff) < new_value < (old_value + max_diff)

for key, value in list(diff.items()): # casting list makes copy, so we don't modify object being iterated.
if isinstance(value, dict):
if "new_value" in value.keys() and "old_value" in value.keys() and _within_tolerance(**value):
diff.pop(key)
else:
self._remove_within_tolerance(diff[key])
self._remove_within_tolerance(diff[key], tolerance)
if not value:
diff.pop(key)


class ParameterMatchType(CheckType):
"""Parameter Match class implementation."""

def evaluate(self, reference_value: Mapping, value_to_compare: Mapping) -> Tuple[Dict, bool]:
@staticmethod
def validate(**kwargs):
"""Method to validate arguments."""
mode_options = ["match", "no-match"]
params = kwargs.get("params")
if not params:
raise ValueError("Params argument is mandatory for ParameterMatch Check Type.")
if not isinstance(params, dict):
raise ValueError(f"Params argument must be a dict, and it's {type(params)}.")

mode = kwargs.get("mode")
if not mode:
raise ValueError("Mode argument is mandatory for ParameterMatch Check Type.")
if not isinstance(mode, str):
raise ValueError(f"Mode argument must be a string, and it's {type(mode)}.")
if mode not in mode_options:
raise ValueError(f"Mode argument should be {mode_options}, and it's {mode}")

def evaluate(self, value_to_compare: Mapping, params: Dict, mode: str) -> Tuple[Dict, bool]:
"""Parameter Match evaluator implementation."""
if not isinstance(value_to_compare, dict):
raise TypeError("check_option must be of type dict()")

evaluation_result = parameter_evaluator(reference_value, value_to_compare)
self.validate(params=params, mode=mode)
# TODO: we don't use the mode?
evaluation_result = parameter_evaluator(value_to_compare, params)
return evaluation_result, not evaluation_result


class RegexType(CheckType):
"""Regex Match class implementation."""

def evaluate(self, reference_value: Mapping, value_to_compare: Mapping) -> Tuple[Mapping, bool]:
@staticmethod
def validate(**kwargs):
"""Method to validate arguments."""
mode_options = ["match", "no-match"]
regex = kwargs.get("regex")
if not regex:
raise ValueError("Params argument is mandatory for Regex Match Check Type.")
if not isinstance(regex, str):
raise ValueError(f"Params argument must be a string, and it's {type(regex)}.")

mode = kwargs.get("mode")
if not mode:
raise ValueError("Mode argument is mandatory for Regex Match Check Type.")
if not isinstance(mode, str):
raise ValueError(f"Mode argument must be a string, and it's {type(mode)}.")
if mode not in mode_options:
raise ValueError(f"Mode argument should be {mode_options}, and it's {mode}")

def evaluate(self, value_to_compare: Mapping, regex: str, mode: str) -> Tuple[Mapping, bool]:
"""Regex Match evaluator implementation."""
# Check that check value_to_compare is dict.
if not isinstance(value_to_compare, dict):
raise TypeError("check_option must be of type dict().")

# Check that value_to_compare has 'regex' and 'mode' dict keys.
if any(key not in value_to_compare.keys() for key in ("regex", "mode")):
raise KeyError(
"Regex check-type requires check-option. Example: dict(regex='.*UNDERLAY.*', mode='no-match')."
)

# Assert that check option has 'regex' and 'mode' dict keys.\
if value_to_compare["mode"] not in ["match", "no-match"]:
raise ValueError(
"Regex check-type requires check-option. Example: dict(regex='.*UNDERLAY.*', mode='no-match')."
)

diff = regex_evaluator(reference_value, value_to_compare)
self.validate(regex=regex, mode=mode)
diff = regex_evaluator(value_to_compare, regex, mode)
return diff, not diff
5 changes: 1 addition & 4 deletions netcompare/evaluators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,14 @@ def parameter_evaluator(values: Mapping, parameters: Mapping) -> Dict:
return result


def regex_evaluator(values: Mapping, parameter: Mapping) -> Dict:
def regex_evaluator(values: Mapping, regex_expression: str, mode: str) -> Dict:
"""Regex Match evaluator engine."""
# values: [{'7.7.7.7': {'peerGroup': 'EVPN-OVERLAY-SPINE'}}]
# parameter: {'regex': '.*UNDERLAY.*', 'mode': 'include'}
result = {}
if not isinstance(values, list):
raise TypeError("Something went wrong during JMSPath parsing. 'values' must be of type List.")

regex_expression = parameter["regex"]
mode = parameter["mode"]

for item in values:
for founded_value in item.values():
for value in founded_value.values():
Expand Down
Loading