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

Pydantic v2 support #210

Open
ManiMozaffar opened this issue Jul 1, 2023 · 25 comments
Open

Pydantic v2 support #210

ManiMozaffar opened this issue Jul 1, 2023 · 25 comments
Labels
enhancement New feature or request

Comments

@ManiMozaffar
Copy link

ManiMozaffar commented Jul 1, 2023

Update: Rocketry seems to not be maintained anymore.
If you're looking for an alternative with supporting pydantic v2, try out AioClock.
https://github.com/ManiMozaffar/aioclock


Is your feature request related to a problem? Please describe.
The problem is that you're using red-bird for logging which is large code base, and also rocketry is dependent on v1.

Describe the solution you'd like

  1. Either create new version of rocketry that matches both pydantic versions, like fastapi did (some dirty trick, but works)
  2. Refactor red bird and rocketry to pin pydantic to v2

Describe alternatives you've considered
None

Additional context
None

@ManiMozaffar ManiMozaffar added the enhancement New feature or request label Jul 1, 2023
@mafrosis
Copy link

mafrosis commented Jul 2, 2023

Alternative, and quickest fix:

  1. pin Pydantic dependency to 1.10.10

@ManiMozaffar
Copy link
Author

Alternative, and quickest fix:

  1. pin Pydantic dependency to 1.10.10

Wouldn't work in my case,
Anyway red-bird isn't even maintained by anyone, doesn't seem very much 'alive' library.
What do you think? @mafrosis

@abuchnick-aiola
Copy link

Same problem here, this issue is blocking me heavily since June 30th.
If it's relevant and could help, I can submit a PR to add the pydantic v2 support to red-bird
@Miksus

@ManiMozaffar
Copy link
Author

Same problem here, this issue is blocking me heavily since June 30th. If it's relevant and could help, I can submit a PR to add the pydantic v2 support to red-bird @Miksus

I didn't notice that red-bird is also maintained by maintainer of this repo,
that's a good news then.
I'll open a PR there, working on Pydantic V2, so hopefully it'll be merged, if not then it should be enough for my own use case at least, to fork rocketry and go with a forked version that supports v2 which uses my own red-bird that supports v2.

@mafrosis
Copy link

mafrosis commented Jul 8, 2023

I think it's up to @Miksus to decide if red-bird is "alive" or not

Simply pin your pydantic version with pydantic==1.10.10. Fixed.

@broven
Copy link

broven commented Jul 14, 2023

Seems I have the similar issue when try intergrate to django followed https://rocketry.readthedocs.io/en/stable/cookbook/django.html

  File "/Users/meta/gitRepos/github.com/broven/ASRHub/.venv/lib/python3.11/site-packages/pydantic/fields.py", line 715, in Field
    raise PydanticUserError('`const` is removed, use `Literal` instead', code='removed-kwargs')
pydantic.errors.PydanticUserError: `const` is removed, use `Literal` instead

For further information visit https://errors.pydantic.dev/2.0.2/u/removed-kwargs

@FrancoTampieri
Copy link

I have the same problem, the fix is ok, but for my project I loose all the benefit of pedantic 2.x

@Jypear
Copy link

Jypear commented Jul 16, 2023

I've started a PR to begin the work integrating pydantic V2 #212
Happy for anybody to contribute with it, I think its going to require a lot of work.

Running locally with a basic 'hello_world' task at the moment, with pydantic 2.0.3 and I've installed @ManiMozaffar red-bird branch where .v1 has been appended to the pydantic imports for now. With Rocketry's dependence on that module, I think that is going to need migrating as well for this to work properly.

@ManiMozaffar
Copy link
Author

I've started a PR to begin the work integrating pydantic V2 #212 Happy for anybody to contribute with it, I think its going to require a lot of work.

Running locally with a basic 'hello_world' task at the moment, with pydantic 2.0.3 and I've installed @ManiMozaffar red-bird branch where .v1 has been appended to the pydantic imports for now. With Rocketry's dependence on that module, I think that is going to need migrating as well for this to work properly.

Heads up: use pydantic migration module, so it’ll replace v1 codes with v2. Then 70 tests will fail and you need to start debugging them. Lots works as you mentioned, that’s why i decided to use it with .v1 because internally it doesn’t use pydantic alot, just as passing by arguments to tasks and etc so I didn’t see a great performance boost.

@Jypear
Copy link

Jypear commented Jul 17, 2023

I've started a PR to begin the work integrating pydantic V2 #212 Happy for anybody to contribute with it, I think its going to require a lot of work.
Running locally with a basic 'hello_world' task at the moment, with pydantic 2.0.3 and I've installed @ManiMozaffar red-bird branch where .v1 has been appended to the pydantic imports for now. With Rocketry's dependence on that module, I think that is going to need migrating as well for this to work properly.

Heads up: use pydantic migration module, so it’ll replace v1 codes with v2. Then 70 tests will fail and you need to start debugging them. Lots works as you mentioned, that’s why i decided to use it with .v1 because internally it doesn’t use pydantic alot, just as passing by arguments to tasks and etc so I didn’t see a great performance boost.

Thanks for the heads up.
Bump script was ran today but it didn't seem to do a whole much. Still working on it, tests are running now atleast.

@Miksus
Copy link
Owner

Miksus commented Jul 18, 2023

Sorry for taking eternity to answer: I should upgrade my machine as developing Rocketry on it is painfully slow with it and have had too busy free time. I'll try to visit this in the week end (and hopefully purchase a new machine).

@Jypear
Copy link

Jypear commented Jul 19, 2023

Thanks @Miksus
I've been working on this the past couple of nights and seem to have it working (for the most part) on v2 apart from one weird bug (probably not a bug, bug).

The way pydantic has changed has caused some odd class inheritance issues, One of the first things I've had to do is comment out creating the null session in the RedBase class in _base.py (Not an ideal solution)
If we don't then we get a clash issue:

Exception has occurred: NameError
Field name "session" shadows an attribute in parent "RedBase"; you might want to use a different field name with "alias='session'".

This issue was also appearing with other classes further down but I managed to get around it by typing those variables to ClassVar based on a issue I found on the pydantic repo pydantic/pydantic#2410 (comment)

This has been fine for getting the application running but when it comes to the test, because it uses DummyTask which inherits Task/FuncTask and then session is passed into it. This seemed to raise the exact same error as above.

I've tried to get around this by setting ClassVar in the dummy class:

class DummyTask(BaseTask):

    session: ClassVar
    def execute(self, *args, **kwargs):
        return

This then causes another error, where when it then tries to run the validation (on name inside the parse_name function) it doesn't seem to pass this attribute up the inheritance into the validation function. So you just end up getting a key error on the test.

FAILED rocketry/test/condition/test_meta.py::test_taskcond_true[main] - KeyError: 'session'

    @field_validator('name', mode="before")
    def parse_name(cls, value, values):
>       session = values.data['session']
E       KeyError: 'session'

# example test that fails

class DummyTask(BaseTask):

    session: ClassVar
    def execute(self, *args, **kwargs):
        return

def test_defaults(session):
    task = DummyTask(name="mytest", session=session)
    assert task.name == "mytest"
    assert isinstance(task.start_cond, AlwaysFalse)
    assert isinstance(task.end_cond, AlwaysFalse)

All in all, I'm not sure what's changed in pydantic to cause that inheritance issue, but if anyone doesn't find a solution it's probably the last main issue with the PR and I can also reimplement the base class into the class structure. It would just need the rest of the warnings working through (But I think on the tests its inheriting a lot of those from RedBird).

@hlafaille
Copy link

Just tried to do a quick fork of this (so I can get back to working) and change the pydantic imports to pydantic.v1, and we need to at least update redbird and tiangolo/pydantic-sqlalchemy.

@Jypear Jypear mentioned this issue Aug 29, 2023
@johnsturgeon
Copy link

Any progress on this?

@Jypear
Copy link

Jypear commented Sep 7, 2023

@johnsturgeon
Will drop in on this to explain where it's up to.
The PR for pydantic-v2 support for Rocketry itself is in a complete state, but it needs reviewing still and the CI fails due to the reliance Rocketry has on Red-Bird (Miksus is the maintainer of this also).

I did create a PR for RedBird also but as it stands I haven't managed to have time to work on it (over the last couple of weeks) and it still needs a lot of work. I would go as far to say that RedBird probably needs more work than the Rocketry stuff did lol.

I'm hoping to try take a look at the RedBird PR this weekend, but I can't guarantee it will be done this weekend.
If anybody has anytime to contribute on the RedBird side I'm more than happy to give them access to work on the fork.

Also when the PRs are done I'm not sure when time would be found to review the code and prep the merge. I'll try to keep people posted on the PR regardless

@johnsturgeon
Copy link

johnsturgeon commented Sep 7, 2023

Thank you for the update.

It's not the end of the world atm (for me at least). The only thing it's holding up for me is an upgrade of a site from Flask to FastAPI (with pydantic 2.0 / beanie). Site still runs great, and I can wait.

Cheers!

@alxmoroz
Copy link

In the meantime I'm leaning towards Celery.

@Kur0x
Copy link

Kur0x commented Oct 28, 2023

pydantic 2 will cause following bug:

Traceback (most recent call last):
  File "/", line 5, in <module>
    from rocketry import Rocketry
  File "/root/miniconda3/lib/python3.10/site-packages/rocketry/__init__.py", line 1, in <module>
    from .session import Session
  File "/root/miniconda3/lib/python3.10/site-packages/rocketry/session.py", line 18, in <module>
    from rocketry.log.defaults import create_default_handler
  File "/root/miniconda3/lib/python3.10/site-packages/rocketry/log/defaults.py", line 1, in <module>
    from redbird.logging import RepoHandler
  File "/root/miniconda3/lib/python3.10/site-packages/redbird/__init__.py", line 2, in <module>
    from .base import BaseRepo, BaseResult
  File "/root/miniconda3/lib/python3.10/site-packages/redbird/base.py", line 116, in <module>
    class BaseRepo(ABC, BaseModel):
  File "/root/miniconda3/lib/python3.10/site-packages/redbird/base.py", line 153, in BaseRepo
    ordered: bool = Field(default=False, const=True)
  File "/root/miniconda3/lib/python3.10/site-packages/pydantic/fields.py", line 763, in Field
    raise PydanticUserError('`const` is removed, use `Literal` instead', code='removed-kwargs')
pydantic.errors.PydanticUserError: `const` is removed, use `Literal` instead

@tekumara
Copy link
Contributor

thanks so much @Jypear i've cloned rocketry #212 and redbird #49 and integrated it at git+https://github.com/tekumara/rocketry@pydantic-v2 and it works well! hoping your PRs can be merged soon 🙏

@Jypear
Copy link

Jypear commented Dec 15, 2023

The only thing to be aware of in that fork is pickling isn't quite working yet (so multiprocess tasks are a bit hit and miss). Pydanticv2 seems to have completely borked it.
Please feel free to have a go at a solution though! I can try and get to it when I have time.

sneakers-the-rat added a commit to sneakers-the-rat/paper-feeds that referenced this issue Jan 6, 2024
Switch to using indexed timestamp as main scraping timestamp
Added rocketry as dep which requires us to pin pydantic<2 because of Miksus/rocketry#210
@Yangeok
Copy link

Yangeok commented Jan 9, 2024

Any progress..?

@Jypear
Copy link

Jypear commented Jan 9, 2024

@Yangeok It's waiting on maintainer still.

@max-pfeiffer
Copy link

+1 here. I just looked at this library and wanted to use it in my FastAPI application (depends on Pydantic v2.0). What a bummer. It looked quite nice...

@RonaldinhoL
Copy link

this project seems inactive now

@ManiMozaffar
Copy link
Author

Update to everyone: Rocketry seems to not be maintained anymore.
So I ended up writing one from scratch.
If you're looking for an alternative with supporting pydantic v2, try out AioClock.
https://github.com/ManiMozaffar/aioclock

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

No branches or pull requests