Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion requirements-rocm.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ tokenizers>=0.15.0
transformers >= 4.36.0 # Required for Mixtral.
fastapi
uvicorn[standard]
pydantic == 1.10.13 # Required for OpenAI server.
pydantic >= 1.9.0, < 3 # Required for OpenAI server.
aioprometheus[starlette]
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ transformers >= 4.36.0 # Required for Mixtral.
xformers == 0.0.23.post1 # Required for CUDA 12.1.
fastapi
uvicorn[standard]
pydantic == 1.10.13 # Required for OpenAI server.
pydantic >= 1.9.0, < 3 # Required for OpenAI server.
aioprometheus[starlette]
2 changes: 1 addition & 1 deletion vllm/entrypoints/openai/api_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def parse_args():
def create_error_response(status_code: HTTPStatus,
message: str) -> JSONResponse:
return JSONResponse(ErrorResponse(message=message,
type="invalid_request_error").dict(),
type="invalid_request_error").model_dump(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is something that was added after 2.0, so won't work for pydantic>=1.9,<2. to fix this, you could instead do from pydantic.v1 import BaseModel and the rest of the file could remain as-is

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the ErrorResponse object is super simple (only has str or None values), we can just let this remain as dict(), as the outcome is the same, without needing any new methods?

Copy link
Contributor

@jvmncs jvmncs Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no dict() method in pydantic v2, per the migration guide. do you mean using the builtin dict function? that would also work here and would allow usage of v2's BaseModel instead of the pydantic.v1 compat class, which seems preferable 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow, pydantic v1 doesn't even have the v1 module. seems pretty strange to not do that for the last few v1 minor versions. anyway looks like dict(ErrorResponse(...)) is the only obvious/clean way to keep the code consistent for v1 & v2 installations. alternative would be to restrict the dependency to >=2,<3

Copy link

@dbuades dbuades Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option, based on your original idea, I suggest you do:

try:
    from pydantic.v1 import BaseModel, Field
except ImportError:
    from pydantic import BaseModel, Field

in https://github.com/vllm-project/vllm/blob/79d64c495463665cf80937a05226806785cfa583/vllm/entrypoints/openai/protocol.py#L6C2-L6C2.

That will work with both versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 @kmcleste can you fix this?

status_code=status_code.value)


Expand Down