feat(parser): use str for SES EmailStr when email_validator package is omitted

[mgochoa]

Issue number: #998

Summary

When email_validator package is not present in the dependencies, pydantic.networks.EmailStr will fail not when imported, but when it's used for typing check. Because inside pydantic.networks.EmailStr, there is a function import_email_validator which will try to import email_validator dinamically.

Changes

In aws_lambda_powertools/utilities/parser/models/ses.py :

has_email_validator = "email_validator" in sys.modules

if has_email_validator:
    from pydantic.networks import EmailStr
else:
    logging.warning("email_validator package is not installed")
    EmailStr = NewType("EmailStr", str)  # type: ignore[no-redef, misc]

It means, now is checking for email_validator package, and defines conditionally which type it should take

User experience

Use could perform:

import pydantic
from aws_lambda_powertools.utilities import batch

without unexpected behavior such as:
ImportError: email-validator is not installed, run 'pip install pydantic[email]'

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of my this change
• Changes are tested
• Changes are documented
• PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

• Migration process documented
• Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[boring-cyborg]

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.

[mgochoa]

This is my first PR on a Open Source tool, If guidance is available I'm open to take what's necessary to make this PR cleaner, and compliant.

[heitorlessa]

Hey @mgochoa thanks a lot for taking the time to add this guard. More importantly, your first PR 🎉 !

without unexpected behavior such as:

This is actually the intended behaviour because when installing parser it'll install email-validator package. As per the issue you referenced, this only happens for customers not following our recommended installation.

That being said, I can see this will continue to happen so best to add this small fix before we tackle it long-term. I'm making two small adjustments and we should be able to merge :)

[heitorlessa]

two comments on checking email_validator being installed vs imported and minor optimizations.

[heitorlessa]

Initialize a logger and change it to debug, so we'll know where it comes from.

logger = logging.getLogger(__name__)

...
logger.debug("email_validator package is not installed; using str instead")

[heitorlessa]

NewType has recently been converted to a class which means it'll have a runtime impact. Since we're looking at a string only IF someone didn't follow our recommended install, use a Type Alias instead.

You can also bring this up to save a else branch, for example:

from pydantic import BaseModel 


EmailStr = str

if has_email_validator:
    from pydantic import EmailStr


class Email(BaseModel):
        address: EmailStr


feedback_channel = Email("[email protected]")
print(feedback_channel.address)

[heitorlessa]

I suspect this will always be False because Pydantic is loading it dynamically, and this will only be true afterwards. You'll need to check if the module is actually installed instead of loaded - I can't recall by heart a 3.6+ compatible way in importlib/pkg_resources to do that. I can help look for that after I finish another doc PR this week

[heitorlessa]

we're gonna need #pragma: nocover too because this will decrease our tests despite being intended behaviour ;-)

[rubenfonseca]

Hi @mgochoa thank you again so much for this PR! Any change you could incorporate the review feedback so we can merge this?

[heitorlessa]

@mgochoa we're going to close this PR and credit you in #1607 for kicking the tires here, as we need to make a V2 release this week.

Thank you!