Skip to content
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

Const validation applied before type validation/parsing #1410

Closed
selimb opened this issue Apr 21, 2020 · 4 comments · Fixed by #1446
Closed

Const validation applied before type validation/parsing #1410

selimb opened this issue Apr 21, 2020 · 4 comments · Fixed by #1446
Labels
bug V1 Bug related to Pydantic V1.X question

Comments

@selimb
Copy link
Contributor

selimb commented Apr 21, 2020

Question

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.5
            pydantic compiled: False
                 install path: D:\dev\github\pydantic\issues\.venv\Lib\site-packages\pydantic
               python version: 3.7.6 (tags/v3.7.6:43364a7ae0, Dec 18 2019, 23:46:00) [MSC v.1916 32 bit (Intel)]
                     platform: Windows-10-10.0.19559-SP0
     optional deps. installed: []
import os
from pathlib import Path
import pydantic

class Config1(pydantic.BaseSettings):
    port: int = pydantic.Field(1234, const=True)

class Config2(pydantic.BaseSettings):
    port: int = pydantic.Field("1234", const=True)

print("No env")
print("Config1", Config1().dict())
print("Config2", Config2().dict())

print("With .env")
Path(".env").write_text("port=1234\n")
print("Config1", Config1(_env_file=".env").dict())  # FAILS
print("Config2", Config2(_env_file=".env").dict())

print("With os.environ")
os.environ["port"] = "1234"
print("Config1", Config1().dict())  # FAILS
print("Config2", Config2().dict())

Copy-paste in the Python REPL to continue on errors:

>>> import os
>>> from pathlib import Path
>>> import pydantic
>>> 
>>> class Config1(pydantic.BaseSettings):
...     port: int = pydantic.Field(1234, const=True)  
...
>>> class Config2(pydantic.BaseSettings):
...     port: int = pydantic.Field("1234", const=True)
...
>>> print("No env")
No env
>>> print("Config1", Config1().dict())
Config1 {'port': 1234}
>>> print("Config2", Config2().dict())
Config2 {'port': 1234}
>>>
>>> print("With .env")
With .env
>>> Path(".env").write_text("port=1234\n")
10
>>> print("Config1", Config1(_env_file=".env").dict())  # FAILS
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "D:\dev\github\pydantic\issues\.venv\lib\site-packages\pydantic\env_settings.py", line 28, in __init__
    super().__init__(**__pydantic_self__._build_values(values, _env_file=_env_file))
  File "D:\dev\github\pydantic\issues\.venv\lib\site-packages\pydantic\main.py", line 338, in __init__
    raise validation_error
pydantic.error_wrappers.ValidationError: 1 validation error for Config1
port
  unexpected value; permitted: 1234 (type=value_error.const; given=1234; permitted=[1234])
>>> print("Config2", Config2(_env_file=".env").dict())
Config2 {'port': 1234}
>>>
>>> print("With os.environ")
With os.environ
>>> os.environ["port"] = "1234"
>>> print("Config1", Config1().dict())  # FAILS
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "D:\dev\github\pydantic\issues\.venv\lib\site-packages\pydantic\env_settings.py", line 28, in __init__
    super().__init__(**__pydantic_self__._build_values(values, _env_file=_env_file))
  File "D:\dev\github\pydantic\issues\.venv\lib\site-packages\pydantic\main.py", line 338, in __init__
    raise validation_error
pydantic.error_wrappers.ValidationError: 1 validation error for Config1
port
  unexpected value; permitted: 1234 (type=value_error.const; given=1234; permitted=[1234])
>>> print("Config2", Config2().dict())
Config2 {'port': 1234}

Looked at the code, and noticed that constant_validator was applied with pre_validators -- before the type validators -- which explains the behaviour.

My question is: is this by design? It looks odd to have the default value be of a different type than the annotation. Additionally, if the value comes from a JSON object instead, then using const means the input type is now constrained to the type of the default value. For instance:

>>> import pydantic
>>>
>>> class Config1(pydantic.BaseSettings):
...     port_const: int = pydantic.Field(1234, const=True)
...     port: int = pydantic.Field(1234)
...
>>> print(Config1())
port_const=1234 port=1234
>>> print(Config1(port_const=1234, port=1234))
port_const=1234 port=1234
>>> print(Config1(port_const="1234", port="1234"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "D:\dev\github\pydantic\issues\.venv\lib\site-packages\pydantic\env_settings.py", line 28, in __init__
    super().__init__(**__pydantic_self__._build_values(values, _env_file=_env_file))
  File "D:\dev\github\pydantic\issues\.venv\lib\site-packages\pydantic\main.py", line 338, in __init__
    raise validation_error
pydantic.error_wrappers.ValidationError: 1 validation error for Config1
port_const
  unexpected value; permitted: 1234 (type=value_error.const; given=1234; permitted=[1234])
@samuelcolvin samuelcolvin added the bug V1 Bug related to Pydantic V1.X label Apr 22, 2020
@samuelcolvin
Copy link
Member

I agree this is wrong.

I can't remember exactly why the const validator is applied as a pre-validator.

If you'd be interested in working on this, feel free to try moving it to a post-validator and see if any tests fail - that might tell you why it had to be a pre-validator.

If we can fix this without backwards incompatible changes, it can go out soon, otherwise it might have to wait until v2.

@selimb
Copy link
Contributor Author

selimb commented Apr 22, 2020

Cool, I'll give it a shot!

Also just came across this:

import pydantic
import os

class Config1(pydantic.BaseSettings):
    port: int = pydantic.Field("1234", const=True)

class Config2(pydantic.BaseModel):
    port: int = pydantic.Field("1234", const=True)


print(Config1())  # 1234
print(Config2())  # '1234'

Pretty sure it's the same problem.

@samuelcolvin
Copy link
Member

That's different, it's because BaseSettings has the validate_all config setting set to True by default.

selimb added a commit to selimb/pydantic that referenced this issue Apr 27, 2020
selimb added a commit to selimb/pydantic that referenced this issue Apr 27, 2020
selimb added a commit to selimb/pydantic that referenced this issue Apr 27, 2020
selimb added a commit to selimb/pydantic that referenced this issue Apr 27, 2020
@selimb
Copy link
Contributor Author

selimb commented Apr 27, 2020

Aaah. Didn't know this config setting existed.

Turns out it didn't break anything! See #1446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V1 Bug related to Pydantic V1.X question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants