From c4beef4a43983616c5b3b5e03107a86301ddc849 Mon Sep 17 00:00:00 2001 From: Cyrus Leung Date: Tue, 23 Apr 2024 10:44:52 +0000 Subject: [PATCH 1/4] Refactor `_load_chat_template` --- vllm/entrypoints/openai/serving_chat.py | 35 +++++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/vllm/entrypoints/openai/serving_chat.py b/vllm/entrypoints/openai/serving_chat.py index d502dd0a4eb7..593de31b713f 100644 --- a/vllm/entrypoints/openai/serving_chat.py +++ b/vllm/entrypoints/openai/serving_chat.py @@ -1,5 +1,6 @@ import codecs import time +from pathlib import Path from typing import AsyncGenerator, AsyncIterator, List, Optional, Union from fastapi import Request @@ -20,6 +21,16 @@ logger = init_logger(__name__) +def _path_is_relative_to(path: Path, other: str): + # Polyfill for Python 3.8 + # TODO: Replace w/ Path.is_relative_to when Python 3.8 is not supported + try: + path.relative_to(*other) + return True + except ValueError: + return False + + class OpenAIServingChat(OpenAIServing): def __init__(self, @@ -319,23 +330,31 @@ async def chat_completion_full_generator( return response def _load_chat_template(self, chat_template): + tokenizer = self.tokenizer + assert tokenizer is not None + if chat_template is not None: try: with open(chat_template, "r") as f: - self.tokenizer.chat_template = f.read() - except OSError: + tokenizer.chat_template = f.read() + except OSError as e: + path = Path(chat_template) + if path.is_absolute() or _path_is_relative_to(path, ''): + msg = (f"The supplied chat template ({chat_template}) " + f"looks like a file path, but it failed to be " + f"opened. Reason: {e}") + raise ValueError(msg) from e + # If opening a file fails, set chat template to be args to # ensure we decode so our escape are interpreted correctly - self.tokenizer.chat_template = codecs.decode( + tokenizer.chat_template = codecs.decode( chat_template, "unicode_escape") logger.info( - f"Using supplied chat template:\n{self.tokenizer.chat_template}" - ) - elif self.tokenizer.chat_template is not None: + f"Using supplied chat template:\n{tokenizer.chat_template}") + elif tokenizer.chat_template is not None: logger.info( - f"Using default chat template:\n{self.tokenizer.chat_template}" - ) + f"Using default chat template:\n{tokenizer.chat_template}") else: logger.warning( "No chat template provided. Chat API will not work.") From 5226efb023c5ad6bec726b80c976c77e9f6fe2e9 Mon Sep 17 00:00:00 2001 From: Cyrus Leung Date: Tue, 23 Apr 2024 11:01:20 +0000 Subject: [PATCH 2/4] Remove unnecessary assert --- vllm/entrypoints/openai/serving_chat.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vllm/entrypoints/openai/serving_chat.py b/vllm/entrypoints/openai/serving_chat.py index 593de31b713f..bca14c2059bd 100644 --- a/vllm/entrypoints/openai/serving_chat.py +++ b/vllm/entrypoints/openai/serving_chat.py @@ -331,7 +331,6 @@ async def chat_completion_full_generator( def _load_chat_template(self, chat_template): tokenizer = self.tokenizer - assert tokenizer is not None if chat_template is not None: try: From 6addaa95425abd5abc43f70e541b6ea29a00bc0c Mon Sep 17 00:00:00 2001 From: Cyrus Leung Date: Tue, 23 Apr 2024 13:31:31 +0000 Subject: [PATCH 3/4] Fix bug in `_path_is_relative_to` --- vllm/entrypoints/openai/serving_chat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vllm/entrypoints/openai/serving_chat.py b/vllm/entrypoints/openai/serving_chat.py index bca14c2059bd..9e280ab7b957 100644 --- a/vllm/entrypoints/openai/serving_chat.py +++ b/vllm/entrypoints/openai/serving_chat.py @@ -25,7 +25,7 @@ def _path_is_relative_to(path: Path, other: str): # Polyfill for Python 3.8 # TODO: Replace w/ Path.is_relative_to when Python 3.8 is not supported try: - path.relative_to(*other) + path.relative_to(other) return True except ValueError: return False From 7464f1d754dd9e8270ddcca494efc45e3ef09618 Mon Sep 17 00:00:00 2001 From: Cyrus Leung Date: Tue, 23 Apr 2024 15:02:06 +0000 Subject: [PATCH 4/4] Update tests; fix the method of detecting filepath-like strings --- tests/async_engine/test_chat_template.py | 19 ++++++++++++++----- vllm/entrypoints/openai/serving_chat.py | 15 ++------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/tests/async_engine/test_chat_template.py b/tests/async_engine/test_chat_template.py index 6972ae1dee4a..8d6ad6706fb0 100644 --- a/tests/async_engine/test_chat_template.py +++ b/tests/async_engine/test_chat_template.py @@ -76,20 +76,29 @@ def test_load_chat_template(): {% if add_generation_prompt and messages[-1]['role'] != 'assistant' %}{{ '<|im_start|>assistant\\n' }}{% endif %}""" # noqa: E501 -def test_no_load_chat_template(): +def test_no_load_chat_template_filelike(): # Testing chatml template template = "../../examples/does_not_exist" tokenizer = MockTokenizer() + mock_serving_chat = MockServingChat(tokenizer) + + with pytest.raises(ValueError, match="looks like a file path"): + OpenAIServingChat._load_chat_template(mock_serving_chat, + chat_template=template) + + +def test_no_load_chat_template_literallike(): + # Testing chatml template + template = "{{ messages }}" + tokenizer = MockTokenizer() + mock_serving_chat = MockServingChat(tokenizer) OpenAIServingChat._load_chat_template(mock_serving_chat, chat_template=template) template_content = tokenizer.chat_template - # Test assertions - assert template_content is not None - # Hard coded value for template_chatml.jinja - assert template_content == """../../examples/does_not_exist""" + assert template_content == template @pytest.mark.asyncio diff --git a/vllm/entrypoints/openai/serving_chat.py b/vllm/entrypoints/openai/serving_chat.py index 9e280ab7b957..2ff335eb7107 100644 --- a/vllm/entrypoints/openai/serving_chat.py +++ b/vllm/entrypoints/openai/serving_chat.py @@ -1,6 +1,5 @@ import codecs import time -from pathlib import Path from typing import AsyncGenerator, AsyncIterator, List, Optional, Union from fastapi import Request @@ -21,16 +20,6 @@ logger = init_logger(__name__) -def _path_is_relative_to(path: Path, other: str): - # Polyfill for Python 3.8 - # TODO: Replace w/ Path.is_relative_to when Python 3.8 is not supported - try: - path.relative_to(other) - return True - except ValueError: - return False - - class OpenAIServingChat(OpenAIServing): def __init__(self, @@ -337,8 +326,8 @@ def _load_chat_template(self, chat_template): with open(chat_template, "r") as f: tokenizer.chat_template = f.read() except OSError as e: - path = Path(chat_template) - if path.is_absolute() or _path_is_relative_to(path, ''): + JINJA_CHARS = "{}\n" + if not any(c in chat_template for c in JINJA_CHARS): msg = (f"The supplied chat template ({chat_template}) " f"looks like a file path, but it failed to be " f"opened. Reason: {e}")