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

Use pydantic 2 #55

Closed
PotHix opened this issue Aug 11, 2023 · 21 comments
Closed

Use pydantic 2 #55

PotHix opened this issue Aug 11, 2023 · 21 comments

Comments

@PotHix
Copy link

PotHix commented Aug 11, 2023

Our codebase uses the latest stable version of pydantic (2.1.1 at the moment) and we're not able to use flagsmith-python-client because one of its dependencies (flagsmith-engine) rely on pydantic <2.

I can see there's a issue for the migration already. Any dates on how flagsmith is going to update pydantic to v2? We will be blocked until the new version is out.

@matthewelwell
Copy link
Contributor

Hi @PotHix, as you've seen, the dependency is in the flagsmith-engine module. We have a PR being worked on here which includes the update of pydantic. We're expecting to complete the PR and release it in about 2-3 weeks (as we have some other commitments until then).

@PotHix
Copy link
Author

PotHix commented Aug 11, 2023

Thanks for replying @matthewelwell! 🙇

That's a huge PR that is much bigger than pydantic. Is there a way to extract the pydantic part and release it before? By reading the issue, the work on pydantic itself feels quite contained (that's all I can guess with little context, though), that's why I'm asking.

If you guys are able to pull this off, it would allow us to use these two weeks to work on the development and migration, otherwise we will be mostly blocked by that.

@matthewelwell
Copy link
Contributor

Hi @PotHix, yes, I appreciate that is a larger PR, however, it is closely tied to the work in that PR. I will try to extract it and put out a release sooner.

@matthewelwell
Copy link
Contributor

Hi @PotHix, just a heads up here that unfortunately there seem to be some hefty breaking changes in pydantic v2, particularly around the use of the pydantic-collections add on. Even when simply upgrading and replacing all imports with from pydantic.v1 ... these errors are seemingly unavoidable.

We will of course continue to complete the upgrade, however, it is not a trivial task. I was hoping that upgrading and just using the pydantic.v1 imports would allow us to complete the upgrade and get you guys moving but it wasn't possible unfortunately.

@PotHix
Copy link
Author

PotHix commented Aug 21, 2023

Hey @matthewelwell! We need some confirmation that you guys can deliver it this week, otherwise, we will have to work on some alternative solutions. Can you please confirm if it's going to be merged this week?

@matthewelwell
Copy link
Contributor

Hi @PotHix, we're planning to look into this on Wednesday. I should be able to provide an update on Wednesday afternoon.

@PotHix
Copy link
Author

PotHix commented Aug 21, 2023

Thanks @matthewelwell. We will be waiting for your answer to decide the next move.

@matthewelwell
Copy link
Contributor

Hi @PotHix, just a quick update here that we're working on this here. We're going to try and have it finished and released by the end of this week but there are a number of breaking changes migrating from pydantic 1 -> 2. Note that the engine is a core piece of technology heavily used in a lot of our services so we will need to have a thorough review and testing process. I will keep you posted on the progress.

@PotHix
Copy link
Author

PotHix commented Aug 23, 2023

Thanks for the update @matthewelwell! If it's guaranteed to have a new version of this library until the end of the week, we can wait for that. If you feel it's uncertain, we would be in a bad situation, as we will have less than 1 week to do everything we need.

Our plan is to have the latest version of the library working on our system on Monday, so we can have the rest of the week to migrate most of our flags from a different provider.

Can we count on this plan or it's too uncertain?

@matthewelwell
Copy link
Contributor

Hi @PotHix, I should have thought about this solution earlier but you can just use flagsmith==3.2.2as that version does not have the pydantic dependency. The interface for the client has not changed - the only feature that you would be missing is offline mode. You can work on integrating the client in the meantime and upgrade to a later version once we have completed the pydantic upgrade. How does that sound?

@PotHix
Copy link
Author

PotHix commented Aug 24, 2023

Thanks for pointing that out! Sounds good. I did some preliminary tests, and it seems to be enough to kickstart the project.

upgrade to a later version once we have completed the pydantic upgrade.

I'm assuming the release of the next version supporting pydantic 2 is going to happen at some point next week. Is that a correct assumption?

@dabeeeenster
Copy link
Contributor

dabeeeenster commented Aug 24, 2023

We cant/don't guarentee any release dates for anything I'm afraid @PotHix! We are working on it and it will come soon! For now you can use the version without the pydantic dependency.

@PotHix
Copy link
Author

PotHix commented Aug 24, 2023

Thanks, @dabeeeenster! We will follow up closely and consider that in our risk. 👍

@PotHix
Copy link
Author

PotHix commented Aug 31, 2023

@matthewelwell I've been facing a problem with Marshmallow when trying to test my implementation when using the version you suggested. It says:

ERROR tests/feature_flags/test_flagsmith.py - marshmallow.warnings.RemovedInMarshmallow4Warning: The 'default' argument to fields is deprecated. Use 'dump_default' instead.
Details A bit more info:
/virtualenv/lib/python3.11/site-packages/flag_engine/environments/schemas.py:38: in <module>
    class BaseEnvironmentSchema(Schema):
        BaseEnvironmentAPIKeySchema = <class 'flag_engine.environments.schemas.BaseEnvironmentAPIKeySchema'>
        EXCLUDE    = 'exclude'
        EnvironmentAPIKeyModel = <class 'flag_engine.environments.models.EnvironmentAPIKeyModel'>
        EnvironmentAPIKeySchema = <class 'flag_engine.environments.schemas.EnvironmentAPIKeySchema'>
        EnvironmentModel = <class 'flag_engine.environments.models.EnvironmentModel'>
        FeatureStateSchema = <class 'flag_engine.features.schemas.FeatureStateSchema'>
        IntegrationSchema = <class 'flag_engine.environments.integrations.schemas.IntegrationSchema'>
        LoadToModelMixin = <class 'flag_engine.utils.marshmallow.schemas.LoadToModelMixin'>
        LoadToModelSchema = <class 'flag_engine.utils.marshmallow.schemas.LoadToModelSchema'>
        ProjectSchema = <class 'flag_engine.projects.schemas.ProjectSchema'>
        Schema     = <class 'marshmallow.schema.Schema'>
        WebhookModel = <class 'flag_engine.environments.models.WebhookModel'>
        WebhookSchema = <class 'flag_engine.environments.schemas.WebhookSchema'>
        __builtins__ = <builtins>
        __cached__ = '/virtualenv/lib/python3.11/site-packages/flag_engine/environments/__pycache__/schemas.cpython-311.pyc'
        __doc__    = None
        __file__   = '/virtualenv/lib/python3.11/site-packages/flag_engine/environments/schemas.py'
        __loader__ = <ddtrace.internal.module._ImportHookChainedLoader object at 0x7fea53f43ad0>
        __name__   = 'flag_engine.environments.schemas'
        __package__ = 'flag_engine.environments'
        __spec__   = ModuleSpec(name='flag_engine.environments.schemas', loader=<ddtrace.internal.module._ImportHookChainedLoader object at 0x7fea53f43ad0>, origin='/virtualenv/lib/python3.11/site-packages/flag_engine/environments/schemas.py')
        fields     = <module 'marshmallow.fields' from '/virtualenv/lib/python3.11/site-packages/marshmallow/fields.py'>
/virtualenv/lib/python3.11/site-packages/flag_engine/environments/schemas.py:41: in BaseEnvironmentSchema
    allow_client_traits = fields.Bool(required=False, default=True)
        __module__ = 'flag_engine.environments.schemas'
        __qualname__ = 'BaseEnvironmentSchema'
        api_key    = <fields.String(dump_default=<marshmallow.missing>, attribute=None, validate=None, required=False, load_only=False, dump_only=False, load_default=<marshmallow.missing>, allow_none=False, error_messages={'required': 'Missing data for required field.', 'null': 'Field may not be null.', 'validator_failed': 'Invalid value.', 'invalid': 'Not a valid string.', 'invalid_utf8': 'Not a valid utf-8 string.'})>
        id         = <fields.Integer(dump_default=<marshmallow.missing>, attribute=None, validate=None, required=False, load_only=False, dump_only=False, load_default=<marshmallow.missing>, allow_none=False, error_messages={'required': 'Missing data for required field.', 'null': 'Field may not be null.', 'validator_failed': 'Invalid value.', 'invalid': 'Not a valid integer.', 'too_large': 'Number too large.'})>
/virtualenv/lib/python3.11/site-packages/marshmallow/fields.py:1176: in __init__
    super().__init__(**kwargs)
        __class__  = <class 'marshmallow.fields.Boolean'>
        falsy      = None
        kwargs     = {'default': True, 'required': False}
        self       = <[AttributeError("'Boolean' object has no attribute 'dump_default'") raised in repr()] Boolean object at 0x7fea53ff5710>
        truthy     = None
/virtualenv/lib/python3.11/site-packages/marshmallow/fields.py:174: in __init__
    warnings.warn(
E   marshmallow.warnings.RemovedInMarshmallow4Warning: The 'default' argument to fields is deprecated. Use 'dump_default' instead.
        additional_metadata = {}
        allow_none = None
        attribute  = None
        data_key   = None
        default    = True
        dump_default = <marshmallow.missing>
        dump_only  = False
        error_messages = None
        load_default = <marshmallow.missing>
        load_only  = False
        metadata   = None
        missing    = <marshmallow.missing>
        required   = False
        self       = <[AttributeError("'Boolean' object has no attribute 'dump_default'") raised in repr()] Boolean object at 0x7fea53ff5710>
        validate   = None

Were you aware of this problem? Is there any workaround?

@matthewelwell
Copy link
Contributor

@PotHix, I'm not really sure how this is throwing an ERROR tbh, that message is a warning only. As you can see from poetry.lock, we are using marshmallow 3.14.0 so the default argument still exists, it's just going to be removed in v4. Do you have some kind of strict warning checker in your testing environment perhaps?

@PotHix
Copy link
Author

PotHix commented Aug 31, 2023

Do you have some kind of strict warning checker in your testing environment perhaps?

We have! I added the warning as an exception, but it seems I did it incorrectly. Now that we considered this warning as expected, things are working again. Thanks for pointing this out!

A have another question about the older version:

you can just use flagsmith==3.2.2as that version does not have the pydantic dependency. The interface for the client has not changed - the only feature that you would be missing is offline mode.

What exactly is the offline mode you mention here? Is it the local evaluation mode?

@matthewelwell
Copy link
Contributor

No, local evaluation mode is available in v3.2.2. The offline mode I'm referring to is not yet documented since we're working on adding it to the rest of our server side SDKs. The description of the requirement can be found here and the PR which added the logic is here.

@khvn26
Copy link
Member

khvn26 commented Nov 23, 2023

Pydantic V2 support released with SDK version 3.5.0.

@khvn26 khvn26 closed this as completed Nov 23, 2023
@levrik
Copy link

levrik commented Aug 22, 2024

I'm basically having the exact opposite issue. I'm stuck on Pydantic v1 due to some technical issues and cannot make use of transient traits as they are only available in newer versions of the Flagsmith SDK which requires Pydantic v2. Maybe flagsmith-engine could be made an optional dependency as I'm not using local evaluation for example.

@matthewelwell
Copy link
Contributor

Hi @levrik, thanks for getting in touch. I'm afraid that this isn't something that we would be looking to add to the client at this stage. I would suggest that you look into whatever other technical constraints are preventing the Pydantic v2 upgrade on your end.

Otherwise, you could always fork this repository and remove the engine yourself if you deem it necessary.

@levrik
Copy link

levrik commented Aug 22, 2024

@matthewelwell I'm trying to upgrade to Pydantic v2 for several months now already. Also a colleague has tried it but didn't manage to do it without breaking something. I guess forking this repo will be the way then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants