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

Update to pydantic 2 #667

Closed
wants to merge 6 commits into from
Closed

Conversation

squidarth
Copy link
Collaborator

@squidarth squidarth commented Sep 18, 2023

Why Pydantic 2?

Pydantic 1 -> 2 upgrade has some serious breaking changes, that have been a big pain for a lot of OSS developers.

The main motivating factor for us updating the version now is that we are going to start using more Pydantic stuff in Truss (particularly around the TrussConfig). We don't want to have to do a migration at some point, so now feels like a good time to make this change.

As a sidenote -- we are already getting complaints from customers that they cannot install truss because it currently depends on pydantic 1.

Impact

It's possible that this will create a new set of customers that cannot install Truss from pypi, which is my biggest worry here. I think there are two main mitigations:

  1. In the immediate-term, we can push people to make a virtualenv to run truss, so that it doesn't impact other python dependencies
  2. In the medium-term, we should move away from a pip install for truss, since it isn't really a Python library (CLI is the main interface now), and instead have it be brew installed primarily.

Alternatives

Supporting both versions of Pydantic

This is a lot of work for us right now for what we are doing.

Notes on implementation

Note that in this branch I don't touch the versions for truss-server or the control-server. The main thing that this impacts is the context builder (and of course, the CLI). Those changes we'll do in a follow-up. It's complicated, because to update fastapi on the control server, we need to have a Truss release first! (the control server depends on Truss).

Testing

I tested building a model on dev, and it seemed to work.

Supporting

@amiruci
Copy link
Member

amiruci commented Sep 18, 2023

i've been reading some stories like this. just a heads up
https://twitter.com/jeremyphoward/status/1703112053446385915

@squidarth
Copy link
Collaborator Author

i've been reading some stories like this. just a heads up
https://twitter.com/jeremyphoward/status/1703112053446385915

@amiruci eThanks for the heads up -- I think as long as it works for our project, i think it's fine. Truss will be becoming just a CLI (and installed via brew), and so I'm not as worried about conflicts with other libs people have installed

@squidarth squidarth marked this pull request as ready for review September 21, 2023 22:33
@bolasim
Copy link
Collaborator

bolasim commented Sep 21, 2023

I don't think it makes sense to bump the versions in pyproject.toml which sets the packages we're using for unit tests, without bumping the versions inside the truss server itself (we'll be testing code in unit tests that's different from what actually gets run). Secondly, this doesn't eliminate the problem of people install pydantic2 in their truss to use for input/output typing.

@squidarth
Copy link
Collaborator Author

I don't think it makes sense to bump the versions in pyproject.toml which sets the packages we're using for unit tests, without bumping the versions inside the truss server itself (we'll be testing code in unit tests that's different from what actually gets run). Secondly, this doesn't eliminate the problem of people install pydantic2 in their truss to use for input/output typing.

That's fair. How about this -- to solve the circular dependency w/ the control server, I can do the following:

  • Make an RC with the changes in this PR
  • Update the truss version in control server to that RC
  • Test this carefully
  • Then after merging this PR, update the truss version in control server requirements to the new proper release

@squidarth
Copy link
Collaborator Author

@squidarth squidarth closed this Sep 27, 2023
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 this pull request may close these issues.

3 participants