Enhance isEmail validation#269
Conversation
There was a problem hiding this comment.
Nice and simple. I wonder:
Is the regex compatible? See comment below.
Is this a feature branch for implementing bufbuild/protovalidate#320?
Yeah sorry, switched the main branch to use a feature branch. Meant to do this last night. |
timostamm
left a comment
There was a problem hiding this comment.
Great. Can you add a docstring for is_email?
def is_email(string: celtypes.Value) -> celpy.Result:
"""Returns true if the string is an email address, for example "foo@example.com".
Conforms to the definition for a valid email address from the HTML standard.
Note that this standard willfully deviates from RFC 5322, which allows many
unexpected forms of email addresses and will easily match a typographical
error.
"""
if not isinstance(string, celtypes.StringType):
msg = "invalid argument, expected string"
raise celpy.CELEvalError(msg)
m = _email_regex.match(string) is not None
return celtypes.BoolType(m)
See https://realpython.com/documenting-python-code/
In most other implementations, we avoid CEL types in the function, and have a small layer for the CEL bindings to converts types. I don't think we should do this in this step, but it might very well turn out to be the right thing to do.
This enhances the current
isEmailvalidation by using a consistent regex that will be used across protovalidate implementations.