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

[Tune] Unpinned pydantic dependency #36990

Closed
yinweisu opened this issue Jun 30, 2023 · 4 comments
Closed

[Tune] Unpinned pydantic dependency #36990

yinweisu opened this issue Jun 30, 2023 · 4 comments
Labels
bug Something that is supposed to be working; but isn't triage Needs triage (eg: priority, bug/not-bug, and owning component)

Comments

@yinweisu
Copy link

What happened + What you expected to happen

pydantic just did a major release, which causes ray tune to return AttributeError: module 'pydantic.fields' has no attribute 'ModelField'

Versions / Dependencies

2.3.1
I've looked at setup.py in the master branch of ray, this issue should be on the latest version too

Reproduction script

NA

Issue Severity

Medium: It is a significant difficulty but I can work around it.

@yinweisu yinweisu added bug Something that is supposed to be working; but isn't triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Jun 30, 2023
@yinweisu
Copy link
Author

yinweisu commented Jun 30, 2023

A more general comment: I see there are many dependencies that are not capped in ray. I would recommend pin them correctly to avoid things like this constantly happen. Customers who consume ray would have to do hot-patches too because of this

@pcmoritz
Copy link
Contributor

pcmoritz commented Jun 30, 2023

Thanks for the bug report, see also #37000

What is the right way to handle these problems depends a lot on the dependency -- some do semantic versioning, some do not, sometimes we need lower pins, sometimes we need upper pins, sometimes we need both -- in general we try to be as lenient as possible since Ray is used with many many different packages and package versions (in fact that's the point of Ray -- to allow people distributing their Python applications).

We are trying to remove dependencies since that is just the best solution for everybody. For packages we cannot remove, we decide on a case-by-case basis what the best way forward is. I agree with your assessment that for pydantic we should put an upper pin on the major version (which will be 3 after we fixed the problems with 2).

See also

#35472
#36609
#36333

and related PRs.

@yinweisu
Copy link
Author

Thanks! I understand your point totally and our project faces similar issues too in a sense that we have tons of dependencies. What we found useful is try to add lower and upper bounds before each release to ensure the environment is being controlled. I know there will be nuance for different cases. But in general, the goal is to avoid an upgrade of a dependency, which is outside what we can control, to break our own package. Glad to see that you guys are working toward that goal!

@woshiyyya
Copy link
Member

woshiyyya commented Jul 3, 2023

Seems that it has been fixed by this PR #37000. I'll close this issue.

edoakes added a commit that referenced this issue Jul 5, 2023
…37097)

This appears to be the same issue as #36990

Pinning the version in the install in `test_backwards_compatibility.sh`
edoakes added a commit to edoakes/ray that referenced this issue Jul 5, 2023
…ay-project#37097)

This appears to be the same issue as ray-project#36990

Pinning the version in the install in `test_backwards_compatibility.sh`

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
bveeramani pushed a commit that referenced this issue Jul 5, 2023
…37097) (#37101)

This appears to be the same issue as #36990

Pinning the version in the install in `test_backwards_compatibility.sh`

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
arvind-chandra pushed a commit to lmco/ray that referenced this issue Aug 31, 2023
…ay-project#37097)

This appears to be the same issue as ray-project#36990

Pinning the version in the install in `test_backwards_compatibility.sh`

Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is supposed to be working; but isn't triage Needs triage (eg: priority, bug/not-bug, and owning component)
Projects
None yet
Development

No branches or pull requests

3 participants