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

Fix IS_PYDANTIC_2 logic for pydantic<1.9.0 #42708

Merged

Conversation

edoakes
Copy link
Contributor

@edoakes edoakes commented Jan 25, 2024

Why are these changes needed?

The __version__ attribute didn't exist prior to 1.9.0 and our check does not work properly.

Tested manually with pip install -U pydantic==1.8.0.

Related issue number

Closes #42413

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

The `__version__` attribute didn't exist prior to `1.9.0` and our check does not work properly.

Tested manually with `pip install -U pydantic==1.8.0`.

---------

Signed-off-by: Edward Oakes <[email protected]>
@architkulkarni
Copy link
Contributor

serve minimal test failed in premerge: https://buildkite.com/ray-project/premerge/builds/17432#018d4208-b02c-4fa4-80ea-a8e50efdac0c

But I think I've seen a similar error before and it was unrelated. Restarting to see what happens

@architkulkarni
Copy link
Contributor

@edoakes serve-minimal failed again, can you see if it's related or if it's safe to ignore?

@GeneDer
Copy link
Contributor

GeneDer commented Jan 25, 2024

@architkulkarni That's actually related to starlette issue. We can cherry pick this PR to unblock from merging #42417 or wait for Sihan's cherry-pick fixes

@architkulkarni
Copy link
Contributor

Ok, let's wait for Sihan's cherry pick fix then. Better to avoid force-merging when possible.

@edoakes
Copy link
Contributor Author

edoakes commented Jan 25, 2024

Ok, let's wait for Sihan's cherry pick fix then. Better to avoid force-merging when possible.

Agreed!

@edoakes
Copy link
Contributor Author

edoakes commented Jan 26, 2024

Other cherry pick that'll fix the test is now open: #42740

@architkulkarni architkulkarni self-assigned this Jan 29, 2024
@architkulkarni architkulkarni merged commit a507eb6 into ray-project:releases/2.9.2 Jan 29, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants