merge-api / merge-python-client

The Python SDK for accessing various Merge Unified APIs
Other
6 stars 10 forks source link

Upgrade to Pydantic 2.0 #82

Open lucasgadams opened 5 months ago

lucasgadams commented 5 months ago

Hi Merge team, Lucas from Materia here. Could you update your library to use pydantic 2.0? Currently it is using the 2.0 library but the 1.0 classes. This actually makes it hard to work with in our system, we need to do a good bit of extra work to convert the objects into things that play properly with our system which is built entirely around 2.0. Pydantic has released great 1.0 -> 2.0 migration scripts (and seems like Fern should just be able to regenerate it all according to pydantic 2.0 spec?). Would be appreciated!

dsinghvi commented 5 months ago

Hey @lucasgadams -- We've intentionally kept the library to use pydantic.v1 so that the SDK can leverage both pydantic V1 and V2 depending on the consumer's dependency. Once Merge no longer cares about supporting v1 users, we can upgrade the dependency.

Would be helpful to understand the specific issues you are running into as you try to leverage the v1 models and we can share solutions on a case-by-case basis.

andersskog commented 3 months ago

Running into some issues caused by mergepythonclient -> Pydantic V1 today -- https://github.com/pydantic/pydantic/issues/9607.

Think this highlights that attempting to migrate to Pydantic 2.0 can be helpful for stability. Our entire backend is blocked from deployment cause of this pydantic V1 dependency.

On some use-cases that we might like to do, one example is to extend a Merge class with fields that are based on other Pydantic models (V2).

lucasgadams commented 3 months ago

While the specific issue noted with pydantic 1.0 not being compatible with python 3.12.4 has been resolved, we are still having broader trouble with the use of v1 pydantic models in our code base. Basically we utilize the merge pydantic objects in various ways, and specifically might wrap them in other pydantic classes or use them as fields in other pydantic classes. Like:

class MergeDocument(BaseModel):  # A v2 pydantic model
    info: File  ## This is merge filestorage.File type, a V1 pydantic model so we have trouble using it as a field here
    integration_type: IntegrationType
    company_id: uuid.UUID

We've migrated our whole codebase to V2, and so incorporating these V1 models presents some challenges.

Andrew-Chen-Wang commented 3 months ago

Hi our deployment cycle broke due to Pydantic 2.0 not being supported. In actuality, it's because Pydantic was upgraded to v1.10.15 in our list of dependencies. This patch was a breaking change in Pydantic, but it also introduced an import error in Merge's SDK. Error log:

    from .account import Account
.venv/lib/python3.10/site-packages/ddtrace/internal/module.py:220: in _exec_module
    self.loader.exec_module(module)
.venv/lib/python3.10/site-packages/merge/resources/accounting/types/account.py:6: in <module>
    from ....core.datetime_utils import serialize_datetime
.venv/lib/python3.10/site-packages/ddtrace/internal/module.py:220: in _exec_module
    self.loader.exec_module(module)
.venv/lib/python3.10/site-packages/merge/core/__init__.py:6: in <module>
    from .jsonable_encoder import jsonable_encoder
.venv/lib/python3.10/site-packages/ddtrace/internal/module.py:220: in _exec_module
    self.loader.exec_module(module)
.venv/lib/python3.10/site-packages/merge/core/jsonable_encoder.py:39: in <module>
    encoders_by_class_tuples = generate_encoders_by_class_tuples(pydantic.json.ENCODERS_BY_TYPE)
E   AttributeError: module 'pydantic.v1' has no attribute 'json'. Did you mean: 'Json'?

By supporting Pydantic v1.10.15, Pydantic v2 may as well be supported as the APIs for v1.10.15 and v2 are compatible. Please support so that we may continue to receive security patches from Pydantic which is core to our services. Thanks

lucasgadams commented 2 months ago

Would really be great to just upgrade the library so that it is pydantic 2.0 by default and instead has some backwards compat for people who are still using v1. Much more ideal than keeping it on V1