From 46eee663435bb91f4929d66b792d1c95421e0ade Mon Sep 17 00:00:00 2001 From: Joseph Catrambone Date: Thu, 8 Aug 2024 12:24:39 -0700 Subject: [PATCH 1/4] Add a safety which maps 'message' as a parameter to error_message. Might be only a bandaid. --- guardrails/classes/validation/validation_result.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/guardrails/classes/validation/validation_result.py b/guardrails/classes/validation/validation_result.py index 18e6a0baa..2985e48a3 100644 --- a/guardrails/classes/validation/validation_result.py +++ b/guardrails/classes/validation/validation_result.py @@ -7,6 +7,7 @@ ErrorSpan as IErrorSpan, ) from guardrails.classes.generic.arbitrary_model import ArbitraryModel +from guardrails.logger import logger class ValidationResult(IValidationResult, ArbitraryModel): @@ -116,6 +117,19 @@ class FailResult(ValidationResult, IFailResult): """ error_spans: Optional[List["ErrorSpan"]] = None + def __init__(self, **kwargs) -> None: + # Prevent a footgun: message is not an expected parameter, error_message is. + # We accept kwargs, but that means a user can pass message by accident and not + # know that it's wrong until they get an exception. + if "error_message" not in kwargs and "message" in kwargs: + logger.warning( + "Parameter missing: FailResult was passed 'message' instead " + "of 'error_message' and 'error_messge' is unspecified." + ) + kwargs["error_message"] = kwargs["message"] + del kwargs["message"] + super().__init__(**kwargs) + @classmethod def from_interface(cls, i_fail_result: IFailResult) -> "FailResult": error_spans = None From 26014a1ad401af3afa01b09101686cb385ef8a63 Mon Sep 17 00:00:00 2001 From: Joseph Catrambone Date: Thu, 8 Aug 2024 12:36:26 -0700 Subject: [PATCH 2/4] Make an explicit constructor for FailResult so that we don't get a footgun late in the game. --- guardrails/classes/validation/validation_result.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/guardrails/classes/validation/validation_result.py b/guardrails/classes/validation/validation_result.py index 18e6a0baa..483f3cb91 100644 --- a/guardrails/classes/validation/validation_result.py +++ b/guardrails/classes/validation/validation_result.py @@ -116,6 +116,12 @@ class FailResult(ValidationResult, IFailResult): """ error_spans: Optional[List["ErrorSpan"]] = None + def __init__(self, error_message: str, **kwargs) -> None: + super().__init__(error_message=error_message, **kwargs) + # This is a silly thing to force a friendly error message and to give type hints + # to IDEs who have a hard time figuring out the constructor parameters. + self.error_message = error_message + @classmethod def from_interface(cls, i_fail_result: IFailResult) -> "FailResult": error_spans = None From 153b4665aa35385e42bb5a925f58bd4e8a30606b Mon Sep 17 00:00:00 2001 From: Joseph Catrambone Date: Thu, 8 Aug 2024 13:58:58 -0700 Subject: [PATCH 3/4] Fix pyright complaint about param being passed to parent. Add to kwargs instead. --- guardrails/classes/validation/validation_result.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/guardrails/classes/validation/validation_result.py b/guardrails/classes/validation/validation_result.py index 483f3cb91..d6427db88 100644 --- a/guardrails/classes/validation/validation_result.py +++ b/guardrails/classes/validation/validation_result.py @@ -117,10 +117,10 @@ class FailResult(ValidationResult, IFailResult): error_spans: Optional[List["ErrorSpan"]] = None def __init__(self, error_message: str, **kwargs) -> None: - super().__init__(error_message=error_message, **kwargs) # This is a silly thing to force a friendly error message and to give type hints # to IDEs who have a hard time figuring out the constructor parameters. - self.error_message = error_message + kwargs["error_message"] = error_message + super().__init__(**kwargs) @classmethod def from_interface(cls, i_fail_result: IFailResult) -> "FailResult": From f21031481d3ac17f7fc8777bb737c3175eee1faf Mon Sep 17 00:00:00 2001 From: Joseph Catrambone Date: Thu, 8 Aug 2024 14:00:10 -0700 Subject: [PATCH 4/4] Revert "Add a safety which maps 'message' as a parameter to error_message. Might be only a bandaid." Replacing with a different approach. This reverts commit 46eee663435bb91f4929d66b792d1c95421e0ade. --- guardrails/classes/validation/validation_result.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/guardrails/classes/validation/validation_result.py b/guardrails/classes/validation/validation_result.py index 2985e48a3..18e6a0baa 100644 --- a/guardrails/classes/validation/validation_result.py +++ b/guardrails/classes/validation/validation_result.py @@ -7,7 +7,6 @@ ErrorSpan as IErrorSpan, ) from guardrails.classes.generic.arbitrary_model import ArbitraryModel -from guardrails.logger import logger class ValidationResult(IValidationResult, ArbitraryModel): @@ -117,19 +116,6 @@ class FailResult(ValidationResult, IFailResult): """ error_spans: Optional[List["ErrorSpan"]] = None - def __init__(self, **kwargs) -> None: - # Prevent a footgun: message is not an expected parameter, error_message is. - # We accept kwargs, but that means a user can pass message by accident and not - # know that it's wrong until they get an exception. - if "error_message" not in kwargs and "message" in kwargs: - logger.warning( - "Parameter missing: FailResult was passed 'message' instead " - "of 'error_message' and 'error_messge' is unspecified." - ) - kwargs["error_message"] = kwargs["message"] - del kwargs["message"] - super().__init__(**kwargs) - @classmethod def from_interface(cls, i_fail_result: IFailResult) -> "FailResult": error_spans = None