diffsync icon indicating copy to clipboard operation
diffsync copied to clipboard

field "model_flags" conflicts with Pydantic 2 protected namespaces

Open dnewood opened this issue 1 year ago • 13 comments

Environment

  • DiffSync version: 2.0.0
  • Python version Python 3.10.12

Observed Behavior

Getting warning logs from Pydantic about protected namespace violations within the DiffSync library

Expected Behavior

No logging output at the warning level

Steps to Reproduce

  1. python3 -i
  2. from diffsync import DiffSyncModel

Extra Information

Pydantic introduces protected namespaces in their new documentation that places warning logs for things like model_ fields documentation here

potential workaround is to use the ConfigDict to set protected_namespaces to an empty value.

class Device(DiffSyncModel):
    """common model used for Diffsync"""
    
    # Disables new Pydantic v2 protections since diffsync uses model_ fields
    model_config = ConfigDict(
        protected_namespaces=()
    )

However, this does not prevent the log entries from the diffsync BaseModel here: https://github.com/networktocode/diffsync/blob/13f5150d66ec76b637f56c908d1ab300bf63661d/diffsync/init.py#L110

dnewood avatar Feb 12 '24 20:02 dnewood

I just tripped over this as well. FWIW, I was able to suppress the warnings by placing this above the Adapter import:

#
# Suppress pydantic warnings
import warnings
warnings.filterwarnings(    # pylint: disable=wrong-import-position
    action="ignore",
    category=UserWarning,
    module="pydantic",
)

#
# Normal code resumes
from diffsync import Adapter

Obvs this should be fixed Correctly™ in diffsync, but at least this silences things.

nrnvgh avatar Aug 17 '24 18:08 nrnvgh

Hi. With v3.0.0, nautobot-app-ssot now moves to diffsync 2.0, making this warning appear and slowing down the process A LOT in nautobot. Any fix in perspective?

dylanbob avatar Aug 30 '24 15:08 dylanbob

slowing down the process A LOT in nautobot.

Can you elaborate what you mean with this? The warning itself shouldn't slow down anything. Can you provide an example with timings?

Kircheneer avatar Sep 03 '24 06:09 Kircheneer

This might be an illusion, but I felt like the very verbose output in the debug console slowed down the process. I do not have any data to back this up, sorry.

dylanbob avatar Sep 03 '24 07:09 dylanbob

@Kircheneer this is related to this error message:

/usr/local/lib/python3.11/site-packages/pydantic/_internal/_fields.py:160: UserWarning: Field "model_flags" has conflict with protected namespace "model_".

You may be able to resolve this warning by setting `model_config['protected_namespaces'] = ()`.

jdrew82 avatar Sep 09 '24 14:09 jdrew82

I've tripped over this as well.

IMO:

  • The best thing to do here is to respect the Pydantic protected namespaces and choose another field name for model_flags. Maybe instance_flags though I'm very open to alternatives -- that's just the first thing that came to mind.
  • A patch should be backwards compatible with model_flags, but provide a deprecation warning
  • I personally would let SemVer guarantees slide a bit since the 2.0 release is so fresh, especially if there's a deprecation warning somewhere.

jamesharr avatar Sep 18 '24 19:09 jamesharr

I did a quick POC of how DiffSync could rename model_flags, keep backwards compatibility, but also generate a useful deprecation warning for DiffSync users.

import warnings
from pydantic import BaseModel, model_validator

class DiffSyncModel(BaseModel):
    instance_flags: str  # NB: use real-type, this is just a place-holder for the proof of concept

    @model_validator(mode="before")
    @classmethod
    def _translate_model_flags(cls, data:any) -> any:
        if isinstance(data, dict) and "model_flags" in data:
            data["instance_flags"] = data["model_flags"]
            del data["model_flags"]
            warnings.warn("model_flags is deprecated, use instance_flags instead", DeprecationWarning,)
        return data

    @property
    def model_flags(self):
        warnings.warn("model_flags is deprecated, use instance_flags instead", DeprecationWarning,)
        return self.instance_flags

    @model_flags.setter
    def model_flags(self, value):
        warnings.warn("model_flags is deprecated, use instance_flags instead", DeprecationWarning,)
        self.instance_flags = value


print("=== No warnings ===")
obj1 = DiffSyncModel(instance_flags="foo")
print(obj1.instance_flags)
obj1.instance_flags = "bar"

print("=== Generates warning at instantiation time ===")
obj2 = DiffSyncModel(model_flags="asdf")

print("=== Generates warning on read ===")
obj3 = DiffSyncModel(instance_flags="foo")
print(obj3.model_flags)

print("=== Generates warning on write ===")
obj4 = DiffSyncModel(instance_flags="foo")
obj1.model_flags = "bar"

This is what's generated when I run the example code:

% python3 dep-warn.py
=== No warnings ===
foo
=== Generates warning at instantiation time ===
/Users/jharr/Downloads/dep-warn.py:13: DeprecationWarning: model_flags is deprecated, use instance_flags instead
  warnings.warn("model_flags is deprecated, use instance_flags instead", DeprecationWarning,)
=== Generates warning on read ===
/Users/jharr/Downloads/dep-warn.py:18: DeprecationWarning: model_flags is deprecated, use instance_flags instead
  warnings.warn("model_flags is deprecated, use instance_flags instead", DeprecationWarning,)
foo
=== Generates warning on write ===
/Users/jharr/Downloads/dep-warn.py:23: DeprecationWarning: model_flags is deprecated, use instance_flags instead
  warnings.warn("model_flags is deprecated, use instance_flags instead", DeprecationWarning,)

The only thing I'm not wild about is using a model_validator in a library like this.

jamesharr avatar Oct 26 '24 04:10 jamesharr

if your custom plugin uses model_flags and nautobot refuses to load it with this error message

22:20:19.081 ERROR nautobot.core.utils.module_loading module_loading.py import_modules_privately() : Unable to load module my_beautiful_plugin from /opt/nautobot/jobs: Field 'model_flags' defined on a base class was overridden by a non-annotated attribute. All field definitions, including overrides, require a type annotation.

For further information visit https://errors.pydantic.dev/2.10/u/model-field-overridden

giving a type to the model_flag fields seems to be the only way to make it happy model_flags: enum.Flag = DiffSyncModelFlags.SKIP_UNMATCHED_DST

gioccher avatar Nov 25 '24 22:11 gioccher

FYI, this might be a useful pyproject.toml snippet for people

[tool.pytest.ini_options]
filterwarnings = [
    # Pydantic 2.x considers DiffSync's model_flags field to be part of a protected
    # namespace; when this is patched, this filter can be removed.
    # https://github.com/networktocode/diffsync/issues/267
    "ignore:Field \"model_flags\" in .* has conflict with protected namespace:UserWarning",
]

jamesharr avatar Mar 07 '25 19:03 jamesharr

FYI, this might be a useful pyproject.toml snippet for people

[tool.pytest.ini_options] filterwarnings = [ # Pydantic 2.x considers DiffSync's model_flags field to be part of a protected # namespace; when this is patched, this filter can be removed. # https://github.com/networktocode/diffsync/issues/267 "ignore:Field "model_flags" in .* has conflict with protected namespace:UserWarning", ]

@jamesharr how would this help if I'd like to remove such errors from my containers console logs? You are referring to a poetry "tool entry" for something pytest related. What helped in our case so far was suppressing the warning like that: https://github.com/networktocode/diffsync/issues/267#issuecomment-2294944274

That on the other hand introduces issues with all sorts of linters (black, isort, ruff, ...), so you get new things you need to handle.

Maybe I don't understand well enough what your pyproject.toml entry is suposed to do.

joj0-biz avatar Mar 25 '25 13:03 joj0-biz

how would this help if I'd like to remove such errors from my containers console logs?

What I posted is a work around to silence warnings during tests run with pytest. It's not a solution. I used that work around as a way to make my tests happy and was hoping in the meantime that diffsync or Pydantic would develop a true solution.

I think a true solution would involve changing the model_flags field to something else or convincing Pydantic that this field is ok.

Better work arounds that function in production were posted above. Specifically the warnings.filterwarnings workaround.

To work around linting and import sorting issues, you might be able to use "noqa" comments or whatever your linter supports.

Again, these are all work arounds and not solutions.

jamesharr avatar Mar 25 '25 13:03 jamesharr

We do intend on finding a more permanent solution for the problem but as it's a breaking change modifying model_flags to something like instance_flags would require a major revision iteration. We have it planned but have some other ideas we'd like to include with it when the change is made.

jdrew82 avatar Mar 25 '25 13:03 jdrew82

silence warnings during tests run with pytest. It's not a solution. I used that work around as a way to make my tests happy and was hoping in the meantime that diffsync or

Hi again, thanks for clarifying. I gave up looking for a real solution a long time ago and understood what you posted might lead to a better workaround for my needs. Currently our only problem is that spam like this interferes with looking for real problems in our Kubernetes and local dev containers.

And yes since I'm about to implement better and automatic linting in our repos, I'm in # noqa: hell already ;-))) Thanks and all the best!

joj0-biz avatar Mar 25 '25 14:03 joj0-biz