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

Support docarray pydantic v2 #6086

Open
JoanFM opened this issue Oct 18, 2023 · 12 comments · May be fixed by #6150
Open

Support docarray pydantic v2 #6086

JoanFM opened this issue Oct 18, 2023 · 12 comments · May be fixed by #6150

Comments

@JoanFM
Copy link
Member

JoanFM commented Oct 18, 2023

Support usage of Jina with docarrat with Pydantic v2.

Aftee new release of docarray, docarray is compatible with the v2 pydantic version. It would be nice to add this compatibility inside Jina.

@MimicTester1307
Copy link

Hi @JoanFM if it's is still needed, I'd like to work on this issue or #5999. I think it'll help familiarize me with the codebase.
Additionally, I tried accessing https://learn.jina.ai/ (I had bookmarked it), but it now redirects to the jina.ai home page. Will the learning resources be up again soon?

@JoanFM
Copy link
Member Author

JoanFM commented Nov 23, 2023

Hey @MimicTester1307 , I do not think these learning resources wlill be back again. Where did you get the link from?

I think you can try to work on the #5999 . This ticket may be complex because we may need to.keep compatibility with 2 pydantic versions at the same time.

@MimicTester1307
Copy link

I see, the courses were pretty comprehensive, though. I think I got it from the JinaAI site initially (a couple of months ago). I bookmarked it for later when I was more comfortable to attempt a contribution.

Alright, thanks. I'll take start with #5999. I can assign it to myself, right?

@JoanFM
Copy link
Member Author

JoanFM commented Nov 23, 2023

I see, the courses were pretty comprehensive, though. I think I got it from the JinaAI site initially (a couple of months ago). I bookmarked it for later when I was more comfortable to attempt a contribution.

I will yes

@Wh1isper
Copy link

Hello, may I ask what is stopping us from upgrading to pydantic v2?

I know there are many integration tests that fail due to upgrades in #6132 . But I think a more beneficial approach would be for jina to upgrade a version number, something like 3.23 -> 3.30, and provide security fixes and important backports for 3.2x only. Once the downstream projects are upgraded, we can stop supporting 3.2x.

This is not in conflict with maintaining Pydantic V1 compatibility, as I know more and more projects are based on Pydantic V2, which forces me to do more complex isolation designs when adopting jina.

@Wh1isper
Copy link

Wh1isper commented Jan 25, 2024

clip-as-service uses docarray, which allows us to interact with clip by way of microservices👍, but clip-as-service's clip_client needs to rely on jina, which results in the project that relies on the clip_client being tied to the pydantic V1, though I just need a client... 😂

I understand that clip_client can't run without jina, so we may should push jina to get with the times.

@JoanFM
Copy link
Member Author

JoanFM commented Jan 25, 2024

Hello, may I ask what is stopping us from upgrading to pydantic v2?

I know there are many integration tests that fail due to upgrades in #6132 . But I think a more beneficial approach would be for jina to upgrade a version number, something like 3.23 -> 3.30, and provide security fixes and important backports for 3.2x only. Once the downstream projects are upgraded, we can stop supporting 3.2x.

This is not in conflict with maintaining Pydantic V1 compatibility, as I know more and more projects are based on Pydantic V2, which forces me to do more complex isolation designs when adopting jina.

Right now, what is blocking is mostly the time availability from my side to dedicate to this feature. If someone from community feels like providing this I would love to help them out however.

@JoanFM JoanFM linked a pull request Mar 7, 2024 that will close this issue
1 task
@jina-bot
Copy link
Member

@jina-ai/product This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 14 days

@jina-bot jina-bot added the Stale label Apr 25, 2024
@TamiasSibiricus
Copy link

@JoanFM I have tried with latest changes in PR #6150 and everything works like a charm except one thing. when i start flow as service in docker container with grpc protocol only sometimes got error with openapi schema generation related to LegacyDocumentJina. Maybe add error report later. Just to be clear this happens only first time when i rebuild image for docker container based on jina 3.25.1 image. Maybe some artefacts.

@JoanFM
Copy link
Member Author

JoanFM commented Apr 25, 2024

Hey @TamiasSibiricus ,

Yes I have observed that, but there is some more nuances about having DocList actually be a List and so on. I hope to get some time to work on that again.

@JoanFM JoanFM removed the Stale label Apr 25, 2024
@TamiasSibiricus
Copy link

@JoanFM have find why openapi schema sometime fails. This happened when LegacyDocumentJina appears in schemas list and fall down in pydantic lib with schema generation on two fields:

class LegacyDocumentJina(BaseDoc):
    ...
    chunks: Optional[Union[DocList[LegacyDocumentJina], List[LegacyDocumentJina]]] = None
    matches: Optional[Union[DocList[LegacyDocumentJina], List[LegacyDocumentJina]]] = None
    ...

Seems that both fields contain doclists with docs refered to self. when i comment this fields in my fork everything works fine.

I just do not car about LegacyDocumentJina fields for backward compatibility BTW.

@JoanFM
Copy link
Member Author

JoanFM commented May 10, 2024

Yes, I also discovered this, but then some new things appear. WIll post more as I find, thanks a lot for the help. If you can do a PR to try all your changes it would be great

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