Fastapi: [FEATURE] Do not log warning if email-validator not installed

[victornoel]

Is your feature request related to a problem? Please describe.

I don't use email validation with pydantic in fastapi and I don't want to have my logs polluted with a warning about it.
This sentiment is also shared in https://github.com/samuelcolvin/pydantic/issues/815

Describe the solution you'd like

There should be no warning logged.

I'm not clear if it's acceptable to just explode when email-validator is missing and an email field must be validated as suggested by https://github.com/samuelcolvin/pydantic/issues/815.

Describe alternatives you've considered

Leaving it like that, it's not the end of the world to have this warning :)

[samuelcolvin]

This check should be unnecessary in fastapi, pydantic will raise an import error when a model is initialised which includes an email field.

Surely they should be sufficient.

[dmontagu]

@samuelcolvin except those email-using objects may be desirable to use (as part of your open api schema) even without validation. I think it makes sense to warn about the removal of validation, but it should be easier to suppress I think.

[dmontagu]

I think something like this might be more appropriate; any thoughts?

from enum import Enum
from typing import Any, Dict, List, Optional, Union
import warnings

from pydantic import BaseModel, Schema as PSchema
from pydantic.types import UrlStr

try:
    import email_validator

    assert email_validator  # make autoflake ignore the unused import
    from pydantic.types import EmailStr  # type: ignore
except ImportError:  # pragma: no cover
    warnings.warn(
        "email-validator not installed, email fields will be treated as str.\n"
        "To install, run: pip install email-validator\n",
        "To suppress this warning, set environment variable "
        "PYTHONWARNINGS=ignore:::fastapi.openapi.models",
        ImportWarning
    )
    class EmailStr(str):  # type: ignore
        pass

...

(Not tested, so may need minor tweaks, but I think the general idea should work.)

[samuelcolvin]

even without validation

It's not validation that raises the ImportError, it's model initialisation. So even if no validation is called, the error will still occur. See here.

[dmontagu]

Yes, sorry my point was just this: rather than adding email-validator as a dependency, I may just want to allow arbitrary strings as emails. But fastapi doesn’t want to do that without any warning to the user, which is why the warning exists.

If you know what is happening and are okay with it though, I think it should be possible to suppress the warning.

[cjw296]

@dmontagu - I think this creates enough noise and confusion that if there's a desire to use EmailStr without email-validator, then the application doing that should take care of catching the ImportError rather than forcing the (what I suspect is the) vast majority of users who don't use EmailStr and have to go out of their way to silence the warning.

[dmontagu]

But it is used by fastapi internally -- is your point that you just don't want fastapi to warn you that it isn't going to validate email addresses if email-validator isn't installed?

I could imagine some edge cases where this results in confusing behavior (e.g., if you add email-validator as a dependency, then something breaks in your fastapi code because you were using an invalid value for an email address somewhere). But that should be so rare that I could get behind dropping the warning entirely -- it mostly just feels right to me to keep the warning "on principle", but practicality may dictate otherwise.

[dmontagu]

For reference: https://github.com/tiangolo/fastapi/blob/55c4b5fb0bd260765b6eedbf8c200caa3cdc670e/fastapi/openapi/models.py#L16

https://github.com/tiangolo/fastapi/blob/55c4b5fb0bd260765b6eedbf8c200caa3cdc670e/fastapi/openapi/models.py#L28

(The above line is the only place where EmailStr occurs.)

Given how inconsequential this is (a single optional field on a rarely used type) I feel like it's probably worth just dropping the warning.

[cjw296]

When are the things in models.py used? It doesn't seem like it's worth doing email validation then, so maybe just change that to be a str and drop the dependency on the package all together?

[dmontagu]

I went through the codebase and I only see one place where a reference to an EmailStr leaves models.py:

• fastapi.models.Contact.email is of type Optional[EmailStr]
• fastapi.models.Info.contact is of type Optional[Contact]
• fastapi.models.OpenAPI.info is of type fastapi.models.Info
• fastapi.models.OpenAPI is used at the end of fastapi.openapi.utils.get_openapi.

The thing is, it is currently not possible to set the info field of the generated OpenAPI to contain a non-None contact usingfastapi.openapi.utils.get_openapi. So I suppose those models are just for convenience if you want to implement your own function to build the OpenAPI instance if overriding fastapi.FastAPI.openapi().

Given all of this, I'd agree that I think it makes sense to drop the dependency.

(Note: the EmailStr type is also currently used in some of the tests, and the tests do actually check that the generated schema says that the field has string format 'email'. So either those tests would need to be changed, or email-validator would need to remain a development dependency. But I'm assuming we're fine with it as a development dependency.)

[cjw296]

email-validator as a test dependency seems sensible, but if we just turn that into a str field then those tests would need to change, right?

[dmontagu]

The tests don't check the schema of the openapi schema type -- they check the schema of an app which uses an unrelated model containing an EmailStr field. So those fields can remain as EmailStr. (There are no tests involving fastapi.models.Contact.)

[cjw296]

Ah, perfect!

[tiangolo]

Thanks for the conversation here everyone!

In a recent version, I added custom logic to log the warning only if the EmailStr is actually used, only if it receives a value: https://github.com/tiangolo/fastapi/pull/946

So, it should not show again, in most of the cases (only if you did custom stuff to include a contact in your OpenAPI).

That is available in latest version.

[cjw296]

I think this issue is good to close :-)

[victornoel]

yep, thanks to all!