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.
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
Update error message for enum validator #673
Conversation
'loc': ('tool',), | ||
'msg': 'value is not a valid enumeration member; permitted: 1, 2', | ||
'type': 'type_error.enum', | ||
'ctx': {'enum_type': ToolEnum}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
putting the type in the context (rather than the permitted values, as in WrongConstantError), means that the type is visible in the traceback for debugging purposes, while only the permitted values are visible in the error message (which may be served externally).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, the idea of ctx
was something like "things you might need when rendering your own error message".
Codecov Report
@@ Coverage Diff @@
## master #673 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 15 15
Lines 2712 2716 +4
Branches 537 538 +1
=====================================
+ Hits 2712 2716 +4 |
LGTM, will merge when I get this far through |
@@ -217,9 +217,10 @@ def set_validator(v: Any) -> Set[Any]: | |||
|
|||
|
|||
def enum_validator(v: Any, field: 'Field', config: 'BaseConfig') -> Enum: | |||
with change_exception(errors.EnumError, ValueError): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't just add kwargs support to context manager?
and then:
with change_exception(errors.EnumError, ValueError, enum_type=field.type_):
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing that would be better placed in a separate PR; not sure it would be worth it if it was only used in this instance. But I haven't looked to see if it could simplify the error handling anywhere else.
Change Summary
Modify the EnumError message to be a little clearer (in line with the WrongConstantError).
Related issue number
#671
Checklist
HISTORY.rst
has been updated#<number>
@<whomever>