-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
fix: tiktoken cache nonroot offline #23498
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
Merged
krrishdholakia
merged 5 commits into
BerriAI:main
from
milan-berri:fix-tiktoken-cache-nonroot
Mar 14, 2026
+56
−44
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b821564
fix: restore offline tiktoken cache for non-root envs
milan-berri 16a06ac
chore: mkdir for custom tiktoken cache dir
milan-berri 3d123f9
test: patch tiktoken.get_encoding in custom-dir test to avoid network
milan-berri b7bb6d6
test: clear CUSTOM_TIKTOKEN_CACHE_DIR in helper for test isolation
milan-berri a6f8389
test: restore default_encoding module state after custom-dir test
milan-berri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,49 +1,57 @@ | ||
| import importlib | ||
| import os | ||
| from unittest.mock import patch | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import litellm.litellm_core_utils.default_encoding as default_encoding | ||
|
|
||
| def test_tiktoken_cache_fallback(monkeypatch): | ||
|
|
||
| def _reload_default_encoding(monkeypatch, **env_overrides): | ||
| """ | ||
| Test that TIKTOKEN_CACHE_DIR falls back to /tmp/tiktoken_cache | ||
| if the default directory is not writable and LITELLM_NON_ROOT is true. | ||
| Helper to reload default_encoding with a clean TIKTOKEN_CACHE_DIR and | ||
| specific environment overrides. | ||
| """ | ||
| # Simulate non-root environment | ||
| monkeypatch.setenv("LITELLM_NON_ROOT", "true") | ||
| monkeypatch.delenv("TIKTOKEN_CACHE_DIR", raising=False) | ||
| monkeypatch.delenv("CUSTOM_TIKTOKEN_CACHE_DIR", raising=False) | ||
| for key, value in env_overrides.items(): | ||
| monkeypatch.setenv(key, value) | ||
| importlib.reload(default_encoding) | ||
|
|
||
| # Mock os.access to return False (not writable) | ||
| # and mock os.makedirs to avoid actually creating /tmp/tiktoken_cache on local machine | ||
| with patch("os.access", return_value=False), patch("os.makedirs"): | ||
| # We need to reload or re-run the logic in default_encoding.py | ||
| # But since it's already executed, we'll just test the logic directly | ||
| # mirroring what we wrote in the file. | ||
|
|
||
| filename = ( | ||
| "/usr/lib/python3.13/site-packages/litellm/litellm_core_utils/tokenizers" | ||
| ) | ||
| is_non_root = os.getenv("LITELLM_NON_ROOT", "").lower() == "true" | ||
|
|
||
| if not os.access(filename, os.W_OK) and is_non_root: | ||
| filename = "/tmp/tiktoken_cache" | ||
| # mock_makedirs(filename, exist_ok=True) | ||
| def test_default_encoding_uses_bundled_tokenizers_by_default(monkeypatch): | ||
| """ | ||
| TIKTOKEN_CACHE_DIR should point at the bundled tokenizers directory | ||
| when no CUSTOM_TIKTOKEN_CACHE_DIR is set, even in non-root environments. | ||
| """ | ||
| _reload_default_encoding(monkeypatch, LITELLM_NON_ROOT="true") | ||
|
|
||
| assert filename == "/tmp/tiktoken_cache" | ||
| assert "TIKTOKEN_CACHE_DIR" in os.environ | ||
| cache_dir = os.environ["TIKTOKEN_CACHE_DIR"] | ||
| assert "tokenizers" in cache_dir | ||
|
|
||
|
|
||
| def test_tiktoken_cache_no_fallback_if_writable(monkeypatch): | ||
| def test_custom_tiktoken_cache_dir_override(monkeypatch, tmp_path): | ||
| """ | ||
| Test that TIKTOKEN_CACHE_DIR does NOT fall back if writable | ||
| CUSTOM_TIKTOKEN_CACHE_DIR must override the default bundled directory | ||
| and the directory should be created if it does not exist. | ||
| Reload with an empty custom dir would otherwise trigger tiktoken to | ||
| download the vocab; we patch get_encoding so the test is offline-safe | ||
| and does not depend on tiktoken's in-memory cache state. | ||
| """ | ||
| monkeypatch.setenv("LITELLM_NON_ROOT", "true") | ||
|
|
||
| filename = "/usr/lib/python3.13/site-packages/litellm/litellm_core_utils/tokenizers" | ||
| custom_dir = tmp_path / "tiktoken_cache" | ||
| with patch( | ||
| "litellm.litellm_core_utils.default_encoding.tiktoken.get_encoding", | ||
| return_value=MagicMock(), | ||
| ): | ||
| _reload_default_encoding( | ||
| monkeypatch, CUSTOM_TIKTOKEN_CACHE_DIR=str(custom_dir) | ||
| ) | ||
|
|
||
| with patch("os.access", return_value=True): | ||
| is_non_root = os.getenv("LITELLM_NON_ROOT", "").lower() == "true" | ||
| if not os.access(filename, os.W_OK) and is_non_root: | ||
| filename = "/tmp/tiktoken_cache" | ||
| cache_dir = os.environ.get("TIKTOKEN_CACHE_DIR") | ||
| assert cache_dir == str(custom_dir) | ||
| assert os.path.isdir(cache_dir) | ||
|
|
||
| assert ( | ||
| filename | ||
| == "/usr/lib/python3.13/site-packages/litellm/litellm_core_utils/tokenizers" | ||
| ) | ||
| # Restore module to a clean state so default_encoding.encoding is a real | ||
| # tiktoken Encoding, not the MagicMock, for any test that runs after this. | ||
| monkeypatch.delenv("TIKTOKEN_CACHE_DIR", raising=False) | ||
| monkeypatch.delenv("CUSTOM_TIKTOKEN_CACHE_DIR", raising=False) | ||
| importlib.reload(default_encoding) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No module-state cleanup after first test
test_default_encoding_uses_bundled_tokenizers_by_defaultcalls_reload_default_encoding, which reloads the module and setsdefault_encoding.encodingto a freshly-loaded tiktokenEncoding. There is no cleanup reload at the end of this test function to restore the module to its original state (unliketest_custom_tiktoken_cache_dir_override, which explicitly reloads at lines 55–57).In practice this is low-risk because the encoding is still a real tiktoken
Encodingobject loaded from the bundled tokenizers, so downstream consumers ofdefault_encoding.encodingwon't receive a broken value. However, if a subsequent test calls_get_default_encoding()in_lazy_imports.pybefore any other reload, it will use the already-cached_default_encoding(set whenlitellmwas first imported) and won't be affected. The concern is ifdefault_encoding.encodingis ever accessed directly (not via the lazy cache), it may differ from the original import-time state.For symmetry and defensive consistency, consider adding a cleanup reload at the end of this test, as done in
test_custom_tiktoken_cache_dir_override:Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!