Skip to content

fix: Unable to route GET requests through proxy - #584

Open
thiagobarbosa wants to merge 1 commit into
openai:mainfrom
thiagobarbosa:fix/get-requests-proxy
Open

fix: Unable to route GET requests through proxy#584
thiagobarbosa wants to merge 1 commit into
openai:mainfrom
thiagobarbosa:fix/get-requests-proxy

Conversation

@thiagobarbosa

Copy link
Copy Markdown

Fix #259

Summary

tiktoken.load.read_file (and the functions that call it) always used the module-level requests.get, so there was no way to route the download requests through a custom requests.Session. This adds an optional session parameter that flows through the whole loading chain, defaulting to None so existing behavior (requests.get) is unchanged when no session is provided.

Changes

  • read_file(blobpath, session=None): uses (session or requests).get(blobpath) instead of always calling requests.get.
  • read_file_cached(blobpath, expected_hash=None, session=None): accepts and forwards session to read_file.
  • data_gym_to_mergeable_bpe_ranks(...): accepts session and forwards it to read_file_cached.
  • load_tiktoken_bpe(tiktoken_bpe_file, expected_hash=None, session=None): accepts session and forwards it to read_file_cached.
  • Added tests/test_load.py with a fake requests.Session to verify read_file uses the provided session instead of the default requests module.

Test plan

  • pytest tests/test_load.py — new test verifies a custom session is used for the HTTP GET instead of the module-level requests.get.
  • Existing behavior preserved: calling read_file/read_file_cached/load_tiktoken_bpe/data_gym_to_mergeable_bpe_ranks without a session still uses requests.get directly.

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.

Unable to route GET requests through proxy

1 participant