Skip to content

fix: accept long JSON strings in ChatTemplateConfig.from_json - #5009

Open
MohammadHijjawi97 wants to merge 1 commit into
InternLM:mainfrom
MohammadHijjawi97:fix/chat-template-from-json-string
Open

MohammadHijjawi97 wants to merge 1 commit into
InternLM:mainfrom
MohammadHijjawi97:fix/chat-template-from-json-string

Conversation

@MohammadHijjawi97

Copy link
Copy Markdown

Motivation

ChatTemplateConfig.from_json is documented to accept "a JSON file or JSON string". It first tries open() on the argument and only falls back to parsing it as JSON on FileNotFoundError. Opening a JSON string as a path can raise other OSErrors, and these are turned into ValueError('Invalid input. Must be a file path or a valid JSON string.') even though the input is valid JSON:

  • Windows: EINVAL for any JSON string, since it contains quotes and colons (reproduced);
  • Linux: ENAMETOOLONG can be raised once the string exceeds the file-name length limit, e.g. a config with a long meta_instruction.
import json
from lmdeploy.model import ChatTemplateConfig
ChatTemplateConfig.from_json(json.dumps({'model_name': 'x', 'meta_instruction': 'You are a helpful assistant. ' * 12}))
# ValueError: Invalid input. Must be a file path or a valid JSON string.

Modification

Read the argument as a file only when os.path.isfile() is true; otherwise parse it as a JSON string. Invalid input still raises a ValueError (json.JSONDecodeError). Added test_chat_template_config_from_json to tests/test_lmdeploy/test_model.py, covering a long JSON string and a JSON file.

BC-breaking (Optional)

No.

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit tests to ensure the correctness.
  3. If the modification has a dependency on downstream projects of a newer version, this PR should be tested with all supported versions of downstream projects.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

from_json tried to open its argument as a file and only treated it as a
JSON string on FileNotFoundError. Opening a JSON string as a path can
raise other OSErrors, such as ENAMETOOLONG on Linux once the string
exceeds the file name limit (e.g. a long meta_instruction), or EINVAL on
Windows for any string containing quotes. Those were turned into
"Invalid input. Must be a file path or a valid JSON string." even though
the input was valid JSON.

Check for an existing file first and otherwise parse the argument as
JSON.

This branch has not been deployed

No deployments
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.

1 participant