Skip to content

Conversation

JosephCatrambone
Copy link
Contributor

@JosephCatrambone JosephCatrambone commented Aug 8, 2024

Possible fix for #997

Before:

In [2]: FailResult(message="Heck")
---------------------------------------------------------------------------
ValidationError                           Traceback (most recent call last)
Cell In[2], line 1
----> 1 FailResult(message="Heck")

File ~/.virtualenvs/guardrails/lib/python3.10/site-packages/pydantic/main.py:176, in BaseModel.__init__(self, **data)
    174 # `__tracebackhide__` tells pytest and some other tools to omit this function from tracebacks
    175 __tracebackhide__ = True
--> 176 self.__pydantic_validator__.validate_python(data, self_instance=self)

ValidationError: 1 validation error for FailResult
error_message
  Field required [type=missing, input_value={'message': 'Heck'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.7/v/missing

After:

>>> FailResult("Heck")
FailResult(outcome='fail', error_message='Heck', fix_value=None, error_spans=None, metadata=None, validated_chunk=None)

>>> FailResult(error_message="Heck")
FailResult(outcome='fail', error_message='Heck', fix_value=None, error_spans=None, metadata=None, validated_chunk=None)

>>> FailResult(message="Heck")
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[4], line 1
----> 1 FailResult(message="Heck")

TypeError: FailResult.__init__() missing 1 required positional argument: 'error_message'

It feels like a bit of a hack to make error_message a required field, since that job really should fall to the BaseModel. This change gives the IDE a type hint (that error_message is the value rather than 'message') and lets people pass a plain string as the first parameter, no named type required.

At some point we should probably shuffle up the stack and remove model_config = ConfigDict(arbitrary_types_allowed=True) from ArbitraryModel, opting for BaseModel where we can.

@JosephCatrambone JosephCatrambone changed the title Add a safety which maps 'message' as a parameter to error_message. Add 'error_message' as an explicitly required parameter into the FailResult to avoid a footgun. Aug 8, 2024
@JosephCatrambone JosephCatrambone marked this pull request as ready for review August 8, 2024 21:06
Copy link
Contributor

@dtam dtam left a comment

Choose a reason for hiding this comment

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

👍🏼 lgtm

@JosephCatrambone JosephCatrambone linked an issue Aug 8, 2024 that may be closed by this pull request
@JosephCatrambone JosephCatrambone merged commit 9e1f4a8 into main Aug 8, 2024
@JosephCatrambone JosephCatrambone deleted the jc/gh_issue_997_validator_help branch August 8, 2024 21:16
@zsimjee zsimjee added this to the v0.5.4 Release Tracker milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] More readable validation failures

3 participants