1.10.2 is broken on some frozen dataclasses

[dolfinus]

Initial Checks

• I have searched GitHub for a duplicate issue and I'm sure this is something new
• I have searched Google & StackOverflow for a solution and couldn't find anything
• I have read and followed the docs and still think this is a bug
• I am confident that the issue is with pydantic (not my code, or another library in the ecosystem like FastAPI or mypy)

Description

After upgrading to pydantic==1.10.2, my code which uses both frozen dataclasses and pydantic models started throwing exceptions:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pydantic/main.py", line 198, in pydantic.main.ModelMetaclass.__new__
  File "pydantic/fields.py", line 506, in pydantic.fields.ModelField.infer
  File "pydantic/fields.py", line 436, in pydantic.fields.ModelField.__init__
  File "pydantic/fields.py", line 557, in pydantic.fields.ModelField.prepare
  File "pydantic/fields.py", line 831, in pydantic.fields.ModelField.populate_validators
  File "pydantic/validators.py", line 725, in find_validators
  File "pydantic/dataclasses.py", line 479, in make_dataclass_validator
    # Make sure we don't have fields without defaults following fields
  File "pydantic/dataclasses.py", line 231, in pydantic.dataclasses.dataclass
    )
  File "pydantic/dataclasses.py", line 218, in pydantic.dataclasses.dataclass.wrap
    # When cls._FIELDS is filled in with a list of Field objects, the name
  File "/usr/local/lib/python3.7/dataclasses.py", line 1010, in dataclass
    return wrap(_cls)
  File "/usr/local/lib/python3.7/dataclasses.py", line 1002, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash, frozen)
  File "/usr/local/lib/python3.7/dataclasses.py", line 879, in _process_class
    raise TypeError('cannot inherit non-frozen dataclass from a '
TypeError: cannot inherit non-frozen dataclass from a frozen one

See example below.

This is caused by #4484 and #4477

Example Code

from dataclasses import dataclass
from pydantic import BaseModel

@dataclass(frozen=True)
class First:
    a: int


@dataclass(frozen=True)
class Second(First):
    @property
    def b(self):
        return self.a

class My(BaseModel):
    my: Second

Python, Pydantic & OS Version

pydantic version: 1.10.2
            pydantic compiled: True
                 install path: /usr/local/lib/python3.7/site-packages/pydantic
               python version: 3.7.7 (default, Oct 21 2021, 12:06:26)  [GCC 4.8.5 20150623 (Red Hat 4.8.5-44.0.3)]
                     platform: Linux-5.19.4-1-MANJARO-x86_64-with-redhat-7.9-Maipo
     optional deps. installed: ['dotenv', 'typing-extensions']

Affected Components

• Compatibility between releases
• Data validation/parsing
• Data serialization - .dict() and .json()
• JSON Schema
• Dataclasses
• Model Config
• Field Types - adding or changing a particular data type
• Function validation decorator
• Generic Models
• Other Model behaviour - construct(), pickling, private attributes, ORM mode
• Settings Management
• Plugins and integration with other tools - mypy, FastAPI, python-devtools, Hypothesis, VS Code, PyCharm, etc.

[dolfinus]

@samuelcolvin @PrettyWood Could you please yank the 1.10.2 release before this behavior will be fixed?

[samuelcolvin]

??? This is not a reason to yank anything.

[samuelcolvin]

any chance this is fixed by #4493?

[PrettyWood]

Nope...I'll take some time tonight to work on this

[dolfinus]

This is not a reason to yank anything.

Minor release bring up some issues which lead to unexpected failures, this is not a reason?

This issue is not connected with __post_init__. It is caused by the second check from this line:

pydantic/pydantic/dataclasses.py

Line 200 in f341049

if is_builtin_dataclass(cls) and _extra_dc_args(_cls) == _extra_dc_args(_cls.__bases__[0]): # type: ignore

Dataclass from the example above is a builtin dataclass, but Second have some methods which are distinct from First. It looks like this check is supposed to get custom validators from a class, but any method/classmethod/propertie/etc is catched too.

This leads to wrapping this dataclass with dataclasses.dataclass decorator, but inner dataclass is frozen when outer is not, so an exception is being raised.

[samuelcolvin]

this is not a reason?

In my opinion yanking is only for mistaken releases (immediately), malicious releases (if someone got your access keys), or very serious security threats e.g. SQL injection or remote code execution.

Yanking a release would break many peoples builds who have pinned to that version, it's reserved for the most serious scenarios.

Also, V1.10.2 contains a number of fixes including partial mitigation of a dos risk, so I'm even less willing to yank it.

[samuelcolvin]

@dolfinus if this issue is urgent for you, why not try to fix it and create a PR? @PrettyWood and I would be happy to review it.

[dolfinus]

I see here 2 options:

1. Revert fix: dataclass wrapper was not always called #4484. That is not good because this PR resolved other issue which affects other users
2. Create PR which will solve current issue, and will not affect solution of __post_init__ and validators are not triggered in class inherited from stdlib dataclass in 1.10.1 #4477. But I'm not aware of pydantic code base, and this could take a lot of time, which is incompatible with urgent status. That's why I asked about yanking release.

[samuelcolvin]

Hi @dolfinus as far as I know, you're the only person for whom this is urgent. So best if you create a pull request. Otherwise you'll need to wait for @PrettyWood to have some time to fix it.

But I would argue that this is not especially urgent since you can use v1.10 or v1.9.2, the error is not silent or security critical unless I'm missing something.

If yanking v1.10.2 would be a satisfactory fix for you, so is pinning to v1.10.1.

[hramezani]

I've checked it a little bit. seems the and _extra_dc_args(_cls) == _extra_dc_args(_cls.__bases__[0]) created the problem.

Question to @PrettyWood: Does it make sense to limit the above check to when we have a custom validation function or __post_init__? if so, how we can find custom validation functions?

[iwanbolzern]

+1 We are facing the same issue. Our workaround is ignoring 1.10.2 but it would be great to have it fixed in the next release. Thanks for all your work!

[dolfinus]

Removing _extra_dc_args(_cls) == _extra_dc_args(_cls.__bases__[0]) fixes the issue, but fails on this test with an error DataclassProxy.__init__() takes 2 positional argument but 4 were given.

This is caused by wrapping dataclass with DataclassProxy, which should perform validation over a dataclass, but bypass all the attributes access and method calls to the dataclass. Wrapping this proxy with another @pydantic.dataclasses.dataclass causes calling the wrong method, and test is failing.

I've tried to replace this proxy object just dataclass itsel with more complex logic in wrappers for __init__ and __post_init__ methods, but other tests were failing, in particular this one and some others, which are checking that builtin dataclass is not changed by adding a decorator over it.

So I stuck with this lump of magic and wrappers over wrappers inside pydantic's dataclass decorator

[di]

In my opinion yanking is only for mistaken releases (immediately), malicious releases (if someone got your access keys), or very serious security threats e.g. SQL injection or remote code execution.

Yanking a release would break many peoples builds who have pinned to that version, it's reserved for the most serious scenarios.

👋 Hi there, PyPI maintainer here, just dropping in to point out that this isn't quite true -- yanking (as described in PEP 592) still leaves the release installable for anyone pinned to it, but for anyone attempting to install without pins would not get it -- effectively, it would no longer be automatically resolved as 'latest'.

Deleting a release does have this effect, and should only be reserved for the cases mentioned.

BTW, thanks for maintaining pydantic! We use it on PyPI (this issue is actually blocking our upgrade to the latest version: pypi/warehouse#12189 😉 )

[samuelcolvin]

Thanks for the hint, I'll try to get onto the next patch release this week.

[PrettyWood]

I'm on holiday starting tomorrow. My plan is to finally take some time to work on those issues.

[samuelcolvin]

Thanks @PrettyWood, I'll be working on v1.10.3 tomorrow too.

[samuelcolvin]

Closed via #4878