Miksus / rocketry

Modern scheduling library for Python
https://rocketry.readthedocs.io
MIT License
3.28k stars 108 forks source link

Pydantic v2 support #210

Open ManiMozaffar opened 1 year ago

ManiMozaffar commented 1 year ago

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

mafrosis commented 1 year ago

Alternative, and quickest fix:

  1. pin Pydantic dependency to 1.10.10
ManiMozaffar commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Jypear commented 1 year ago

I've started a PR to begin the work integrating pydantic V2 https://github.com/Miksus/rocketry/pull/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/pydantic/pydantic/discussions/2410#discussioncomment-408613

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 commented 1 year ago

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.

johnsturgeon commented 1 year ago

Any progress on this?

Jypear commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

In the meantime I'm leaning towards Celery.

Kur0x commented 1 year ago

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 commented 11 months ago

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 commented 11 months ago

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.

Yangeok commented 10 months ago

Any progress..?

Jypear commented 10 months ago

@Yangeok It's waiting on maintainer still.

max-pfeiffer commented 9 months ago

+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 commented 9 months ago

this project seems inactive now

ManiMozaffar commented 6 months ago

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