Skip to content

Conversation

@smaye81
Copy link
Contributor

@smaye81 smaye81 commented Mar 23, 2025

This enhances the current isEmail validation by using a consistent regex that will be used across protovalidate implementations.

Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

Nice and simple. I wonder:
Is the regex compatible? See comment below.
Is this a feature branch for implementing bufbuild/protovalidate#320?

@smaye81 smaye81 changed the base branch from main to sayers/validation_upgrades March 24, 2025 14:22
@smaye81
Copy link
Contributor Author

smaye81 commented Mar 24, 2025

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.

Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

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 "[email protected]".

    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.

@smaye81 smaye81 merged commit 52cf9a2 into sayers/validation_upgrades Mar 24, 2025
6 checks passed
@smaye81 smaye81 deleted the sayers/email_validation branch March 24, 2025 16:01
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.

3 participants