Upgrade to run against OpenAI >= 1.0

[simonw]

Refs:

• Upgrade for compatibility with OpenAI 1.0 library #325

Original goal was "Upgrade to run against both OpenAI < 1.0 and OpenAI >= 1.0" but I eventually determined that the changes between the two were just too extensive for it to be worth trying to build a compatibility shim.

[simonw]

Looking at this diff generated by openai upgrade:

diff --git a/llm/default_plugins/openai_models.py b/llm/default_plugins/openai_models.py
index 611340d..d787c7a 100644
--- a/llm/default_plugins/openai_models.py
+++ b/llm/default_plugins/openai_models.py
@@ -4,6 +4,9 @@ from llm.utils import dicts_to_table_string
 import click
 import datetime
 import openai
+from openai import OpenAI
+
+client = OpenAI()
 import os
 
 try:
@@ -22,7 +25,7 @@ if os.environ.get("LLM_OPENAI_SHOW_RESPONSES"):
         click.echo(response.text, err=True)
         return response
 
-    openai.requestssession = requests.Session()
+    raise Exception("The 'openai.requestssession' option isn't read in the client API. You will need to pass it when you instantiate the client, e.g. 'OpenAI(requestssession=requests.Session())'")
     openai.requestssession.hooks["response"].append(log_response)
 
 
@@ -111,7 +114,7 @@ class OpenAIEmbeddingModel(EmbeddingModel):
         }
         if self.dimensions:
             kwargs["dimensions"] = self.dimensions
-        results = openai.Embedding.create(**kwargs)["data"]
+        results = client.embeddings.create(**kwargs)["data"]
         return ([float(r) for r in result["embedding"]] for result in results)
 
 
@@ -305,12 +308,10 @@ class Chat(Model):
         response._prompt_json = {"messages": messages}
         kwargs = self.build_kwargs(prompt)
         if stream:
-            completion = openai.ChatCompletion.create(
-                model=self.model_name or self.model_id,
-                messages=messages,
-                stream=True,
-                **kwargs,
-            )
+            completion = client.chat.completions.create(model=self.model_name or self.model_id,
+            messages=messages,
+            stream=True,
+            **kwargs)
             chunks = []
             for chunk in completion:
                 chunks.append(chunk)
@@ -319,12 +320,10 @@ class Chat(Model):
                     yield content
             response.response_json = combine_chunks(chunks)
         else:
-            completion = openai.ChatCompletion.create(
-                model=self.model_name or self.model_id,
-                messages=messages,
-                stream=False,
-                **kwargs,
-            )
+            completion = client.chat.completions.create(model=self.model_name or self.model_id,
+            messages=messages,
+            stream=False,
+            **kwargs)
             response.response_json = completion.to_dict_recursive()
             yield completion.choices[0].message.content
 
@@ -384,12 +383,10 @@ class Completion(Chat):
         response._prompt_json = {"messages": messages}
         kwargs = self.build_kwargs(prompt)
         if stream:
-            completion = openai.Completion.create(
-                model=self.model_name or self.model_id,
-                prompt="\n".join(messages),
-                stream=True,
-                **kwargs,
-            )
+            completion = client.completions.create(model=self.model_name or self.model_id,
+            prompt="\n".join(messages),
+            stream=True,
+            **kwargs)
             chunks = []
             for chunk in completion:
                 chunks.append(chunk)
@@ -398,12 +395,10 @@ class Completion(Chat):
                     yield content
             response.response_json = combine_chunks(chunks)
         else:
-            completion = openai.Completion.create(
-                model=self.model_name or self.model_id,
-                prompt="\n".join(messages),
-                stream=False,
-                **kwargs,
-            )
+            completion = client.completions.create(model=self.model_name or self.model_id,
+            prompt="\n".join(messages),
+            stream=False,
+            **kwargs)
             response.response_json = completion.to_dict_recursive()
             yield completion.choices[0]["text"]

It basically all comes down to these differences:

openai.Embedding.create(...)
client.embeddings.create(...)

openai.ChatCompletion.create(...)
client.chat.completions.create(...)

openai.Completion.create(...)
client.completions.create(...)

So... I could add a get_openai() helper function of some sort that papers over them.

[simonw]

I'm going to do this:

client = get_openai_client(...)
client.ChatCompletion.create(...)
client.Completion.create(...)
client.Embedding.create(...)

Where my special client class knows to proxy to chat.completions.create(...) when necessary.

[simonw]

Tests are now failing as intended - the 1.0 ones fail, the <1.0 ones pass:

[simonw]

On the latest version:

>>> openai.version.VERSION
'1.10.0'
>>> openai.version.VERSION.split('.')[0]
'1'

And on the pre-1.0 version:

>>> import openai
>>> openai.version.VERSION
'0.28.1'
>>> openai.version.VERSION.split('.')[0]
'0'

I can use that to detect the old version.

[simonw]

Uh-oh, I need to convince mypy not to get upset:

[simonw]

llm 'hello world'

Error: Completions.create() got an unexpected keyword argument 'api_key'

I'm going to have to get my get_openai_client() method to take that API key.

[simonw]

Oh this is annoying. I've been passing api_base and so on to the create() method:

llm/llm/default_plugins/openai_models.py

Lines 331 to 355 in 469644c

	def build_kwargs(self, prompt):
	kwargs = dict(not_nulls(prompt.options))
	json_object = kwargs.pop("json_object", None)
	if "max_tokens" not in kwargs and self.default_max_tokens is not None:
	kwargs["max_tokens"] = self.default_max_tokens
	if self.api_base:
	kwargs["api_base"] = self.api_base
	if self.api_type:
	kwargs["api_type"] = self.api_type
	if self.api_version:
	kwargs["api_version"] = self.api_version
	if self.api_engine:
	kwargs["engine"] = self.api_engine
	if json_object:
	kwargs["response_format"] = {"type": "json_object"}
	if self.needs_key:
	if self.key:
	kwargs["api_key"] = self.key
	else:
	# OpenAI-compatible models don't need a key, but the
	# openai client library requires one
	kwargs["api_key"] = "DUMMY_KEY"
	if self.headers:
	kwargs["headers"] = self.headers
	return kwargs

But it looks like I need to pass those to that new constructor instead:

client=OpenAI(
    api_key="<key>",
    base_url="<base_url>",
    http_client=httpx.Client(
        headers={
            "header": "<key>",
			...
        } 
    )
)

This is going to REALLY mess up my compatibility shim.

[simonw]

I think I have two sensible options:

1. Give up on compatibility with pre 1.0 - which is actually pretty reasonable, the chances that someone will have LLM installed in an environment alongside another library which itself depends on OpenAI <1.0 is pretty small
2. Rewrite my core code to call the new OpenAI() constructor and use client.chat.completions.create(...) and friends, then build my own compatibility shim for that instead

I like option 2 a lot - it's much nicer for me to write code against the new API and have a compatibility shim, since I can then easily drop the shim later on if I change my mind.

I'm going to spike on 2 for a bit and, if I can't get that working, switch to option 1 and drop <1.0 entirely.

[simonw]

Urgh:

llm 'hello world'                     
Error: 'ChatCompletionChunk' object is not subscriptable

From the stack trace:

  File "/Users/simon/Dropbox/Development/llm/llm/cli.py", line 277, in prompt
    for chunk in response:
  File "/Users/simon/Dropbox/Development/llm/llm/models.py", line 91, in __iter__
    for chunk in self.model.execute(
  File "/Users/simon/Dropbox/Development/llm/llm/default_plugins/openai_models.py", line 342, in execute
    content = chunk["choices"][0].get("delta", {}).get("content")
TypeError: 'ChatCompletionChunk' object is not subscriptable

[simonw]

Debugger exploration:

-> content = chunk["choices"][0].get("delta", {}).get("content")
(Pdb) chunk
ChatCompletionChunk(id='chatcmpl-8l6l41lMUl3DiI3n5ujjGUF5nlOny', choices=[Choice(delta=ChoiceDelta(content='', function_call=None, role='assistant', tool_calls=None), finish_reason=None, index=0, logprobs=None)], created=1706238086, model='gpt-4-1106-preview', object='chat.completion.chunk', system_fingerprint='fp_f71eafccde')
(Pdb) chunk["choices"]
*** TypeError: 'ChatCompletionChunk' object is not subscriptable
(Pdb) chunk.choices
[Choice(delta=ChoiceDelta(content='', function_call=None, role='assistant', tool_calls=None), finish_reason=None, index=0, logprobs=None)]
(Pdb) chunk.choices[0]
Choice(delta=ChoiceDelta(content='', function_call=None, role='assistant', tool_calls=None), finish_reason=None, index=0, logprobs=None)
(Pdb) chunk.choices[0].get("delta")
*** AttributeError: 'Choice' object has no attribute 'get'
(Pdb) chunk.choices[0].delta
ChoiceDelta(content='', function_call=None, role='assistant', tool_calls=None)
(Pdb) chunk.choices[0].delta.content
''

[simonw]

I'm going to give up on the <1.0 dream at this point, the changes are just too deep for it to be worth trying to paper over them with a compatibility shim.

[simonw]

Well here's a painful thing...

(Pdb) completion.__class__
<class 'openai.types.chat.chat_completion.ChatCompletion'>
(Pdb) completion.__class__.__bases__
(<class 'openai._models.BaseModel'>,)
(Pdb) completion.__class__.__bases__[0]
<class 'openai._models.BaseModel'>
(Pdb) completion.__class__.__bases__[0].__bases__
(<class 'pydantic.main.BaseModel'>,)

It turns out the new OpenAI library uses Pydantic. I've had huge pain keeping LLM compatible with Pydantic 1 and Pydantic 2 already!

But... https://github.com/openai/openai-python/blob/0c1e58d511bd60c4dd47ea8a8c0820dc2d013d1d/pyproject.toml#L12 says

dependencies = [
    "httpx>=0.23.0, <1",
    "pydantic>=1.9.0, <3",

So maybe OpenAI is compatible with both Pydantic versions itself?

Yes, it turns out there are! Here's their own Pydantic 1 v. 2 compatibility shim: https://github.com/openai/openai-python/blob/0c1e58d511bd60c4dd47ea8a8c0820dc2d013d1d/src/openai/_compat.py#L20

[simonw]

From openai/openai-python@v0.28.1...v1.0.0 I confirmed that pydantic was indeed one of the new things they added in 1.0.

[simonw]

Now my tests are failing and I'm worried it might be because my mocks don't work any more:

        result = runner.invoke(cli, ["hello", "--no-stream"], catch_exceptions=False)
>       assert result.exit_code == 0
E       assert 1 == 0
E        +  where 1 = <Result SystemExit(1)>.exit_code

tests/test_keys.py:68: AssertionError
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> entering PDB >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB post_mortem (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /Users/simon/Dropbox/Development/llm/tests/test_keys.py(68)test_uses_correct_key()
-> assert result.exit_code == 0
(Pdb) result
<Result SystemExit(1)>
(Pdb) result.stdout
"Error: Error code: 401 - {'error': {'message': 'Incorrect API key provided: from-key**file. You can find your API key at https://platform.openai.com/account/api-keys.', 'type': 'invalid_request_error', 'param': None, 'code': 'invalid_api_key'}}\n"

Did 1.0 change the HTTP library they use? Yes - it looks like they switched from requests to httpx.

[simonw]

openai/openai-python#742 confirms that openai.requestssession has been removed. So this code is obsolete:

llm/llm/default_plugins/openai_models.py

Lines 20 to 26 in 53c845e

	def _log_response(response, *args, **kwargs):
	click.echo(response.text, err=True)
	return response
	_log_session = requests.Session()
	_log_session.hooks["response"].append(_log_response)

LLM_OPENAI_SHOW_RESPONSES is a documented feature. If I remove it I need to update the documentation.

[simonw]

Spotted in https://github.com/openai/openai-python/blob/0c1e58d511bd60c4dd47ea8a8c0820dc2d013d1d/src/openai/_utils/_logs.py#L5

def setup_logging() -> None:
    env = os.environ.get("OPENAI_LOG")
    if env == "debug":
        _basic_config()
        logger.setLevel(logging.DEBUG)
        httpx_logger.setLevel(logging.DEBUG)
    elif env == "info":
        _basic_config()
        logger.setLevel(logging.INFO)
        httpx_logger.setLevel(logging.INFO)

So maybe I can ditch LLM_OPENAI_SHOW_RESPONSES in favour of OPENAI_LOG instead?

[simonw]

Here's what that does:

$ OPENAI_LOG=debug llm 'hello'
[2024-01-25 19:34:46 - httpx:79 - DEBUG] load_ssl_context verify=True cert=None trust_env=True http2=False
[2024-01-25 19:34:46 - httpx:146 - DEBUG] load_verify_locations cafile='/Users/simon/.local/share/virtualenvs/llm-p4p8CDpq/lib/python3.10/site-packages/certifi/cacert.pem'
[2024-01-25 19:34:46 - openai._base_client:439 - DEBUG] Request options: {'method': 'post', 'url': '/chat/completions', 'files': None, 'json_data': {'messages': [{'role': 'user', 'content': 'hello'}], 'model': 'gpt-4-1106-preview', 'stream': True}}
[2024-01-25 19:34:46 - httpx:1013 - INFO] HTTP Request: POST https://api.openai.com/v1/chat/completions "HTTP/1.1 200 OK"
[2024-01-25 19:34:46 - openai._base_client:934 - DEBUG] HTTP Request: POST https://api.openai.com/v1/chat/completions "200 OK"
Hello! How can I assist you today?
$ OPENAI_LOG=info llm 'hello'
[2024-01-25 19:34:56 - httpx:1013 - INFO] HTTP Request: POST https://api.openai.com/v1/chat/completions "HTTP/1.1 200 OK"
Hello! How can I assist you today?

That's good enough, I'll make that change.

[simonw]

It's not as good - you don't get the full response, which is the thing that was most useful. But maybe I'll add that back in again in the future.

[simonw]

Got this test failure:

>       assert json.loads(row["response_json"]) == {
            "model": "gpt-3.5-turbo",
            "usage": {},
            "choices": [{"message": {"content": "Bob, Alice, Eve"}}],
        }

The debugger shows:

(Pdb) pprint(json.loads(row["response_json"]))
{'choices': [{'finish_reason': None,
              'index': None,
              'logprobs': None,
              'message': {'content': 'Bob, Alice, Eve',
                          'function_call': None,
                          'role': None,
                          'tool_calls': None}}],
 'created': None,
 'id': None,
 'model': 'gpt-3.5-turbo',
 'object': None,
 'system_fingerprint': None,
 'usage': {'completion_tokens': None,
           'prompt_tokens': None,
           'total_tokens': None}}

[simonw]

The problem here is that I used to store the exact JSON that came back from the API - but the new Pydantic layer in OpenAI 1.0 reshapes that original information into something a lot more verbose, with a ton of None values.

[simonw]

Pushing what I have so far.

[simonw]

I'm going to clean that up with this:

def remove_dict_none_values(d: dict) -> dict:
    """
    Recursively remove keys with value of None or value of a dict that is all values of None
    """
    if not isinstance(d, dict):
        return d
    new_dict = {}
    for key, value in d.items():
        if value is not None:
            if isinstance(value, dict):
                nested = remove_dict_none_values(value)
                if nested:
                    new_dict[key] = nested
            elif isinstance(value, list):
                new_dict[key] = [remove_dict_none_values(v) for v in value]
            else:
                new_dict[key] = value
    return new_dict

Written with help of: https://chat.openai.com/share/0dbf00c3-3feb-423c-98aa-7a4cca89023c

[simonw]

One last error:

(Pdb) result.output
"Error: OpenAI.__init__() got an unexpected keyword argument 'api_base'\n"

From this test:

llm/tests/test_llm.py

Lines 460 to 496 in 469644c

	EXTRA_MODELS_YAML = """
	- model_id: orca
	model_name: orca-mini-3b
	api_base: "http://localai.localhost"
	- model_id: completion-babbage
	model_name: babbage
	api_base: "http://localai.localhost"
	completion: 1
	"""
	def test_openai_localai_configuration(mocked_localai, user_path):
	log_path = user_path / "logs.db"
	sqlite_utils.Database(str(log_path))
	# Write the configuration file
	config_path = user_path / "extra-openai-models.yaml"
	config_path.write_text(EXTRA_MODELS_YAML, "utf-8")
	# Run the prompt
	runner = CliRunner()
	prompt = "three names \nfor a pet pelican"
	result = runner.invoke(cli, ["--no-stream", "--model", "orca", prompt])
	assert result.exit_code == 0
	assert result.output == "Bob, Alice, Eve\n"
	assert json.loads(mocked_localai.last_request.text) == {
	"model": "orca-mini-3b",
	"messages": [{"role": "user", "content": "three names \nfor a pet pelican"}],
	"stream": False,
	}
	# And check the completion model too
	result2 = runner.invoke(cli, ["--no-stream", "--model", "completion-babbage", "hi"])
	assert result2.exit_code == 0
	assert result2.output == "Hello\n"
	assert json.loads(mocked_localai.last_request.text) == {
	"model": "babbage",
	"prompt": "hi",
	"stream": False,
	}

openai/openai-python#742 says:

openai.api_base -> openai.base_url

[simonw]

Well they passed on my laptop! Failing here because I forgot to run linters.

[simonw]

I used ChatGPT to help port the requests mock tests to pytest: https://chat.openai.com/share/1ea97304-1ceb-4e4c-9213-bae9949cd261

[simonw]

Turns out Black was failing because I needed to upgrade Black in my dev environment (as of only 45 minutes ago: https://github.com/psf/black/releases/tag/24.1.0)

Installing collected packages: black
  Attempting uninstall: black
    Found existing installation: black 23.7.0
    Uninstalling black-23.7.0:
      Successfully uninstalled black-23.7.0
Successfully installed black-24.1.0

diff --git a/llm/cli.py b/llm/cli.py
index 3fa2ecc..03a6b09 100644
--- a/llm/cli.py
+++ b/llm/cli.py
@@ -746,12 +746,16 @@ def logs_list(
             click.echo(
                 "# {}{}\n{}".format(
                     row["datetime_utc"].split(".")[0],
-                    "    conversation: {}".format(row["conversation_id"])
-                    if should_show_conversation
-                    else "",
-                    "\nModel: **{}**\n".format(row["model"])
-                    if should_show_conversation
-                    else "",
+                    (
+                        "    conversation: {}".format(row["conversation_id"])
+                        if should_show_conversation
+                        else ""
+                    ),
+                    (
+                        "\nModel: **{}**\n".format(row["model"])
+                        if should_show_conversation
+                        else ""
+                    ),
                 )
             )
             # In conversation log mode only show it for the first one
diff --git a/llm/embeddings.py b/llm/embeddings.py
index 8c5333f..5efeda0 100644
--- a/llm/embeddings.py
+++ b/llm/embeddings.py
@@ -220,12 +220,12 @@ class Collection:
                             "collection_id": collection_id,
                             "id": id,
                             "embedding": llm.encode(embedding),
-                            "content": value
-                            if (store and isinstance(value, str))
-                            else None,
-                            "content_blob": value
-                            if (store and isinstance(value, bytes))
-                            else None,
+                            "content": (
+                                value if (store and isinstance(value, str)) else None
+                            ),
+                            "content_blob": (
+                                value if (store and isinstance(value, bytes)) else None
+                            ),
                             "content_hash": self.content_hash(value),
                             "metadata": json.dumps(metadata) if metadata else None,
                             "updated": int(time.time()),

That's a nice cosmetic improvement.

[rattrayalex]

Heya @simonw, I'm one of the maintainers of the openai package. Another maintainer, @RobertCraigie, sent me this thread after reading through it. Tremendous thanks for the detailed stream-of-consciousness! Very useful.

Few follow-ups:

[OPENAI_LOG is] not as good - you don't get the full response, which is the thing that was most useful. But maybe I'll add that back in again in the future.

This is good feedback, we may add that in the future. In the meantime, you can use httpx's event_hooks to achieve this if you like.

the new Pydantic layer in OpenAI 1.0 reshapes that original information into something a lot more verbose, with a ton of None values

This is good feedback – we've been torn between sticking with Pydantic defaults (what you see here, lots of Nones) and something more minimally JSONic (what you expected). Do you have any advice or suggestions here?

Note that I believe that your remove_dict_none_values could be replaced with foo.model_dump_json(indent=2, exclude_unset=True), as documented here. We're considering adding a .to_json() helper method which would do this for you more conveniently.

EDIT: I should ask, do you have any other feedback, whether overall / high-level, or nitty-gritty, about how the python SDK could be better or how the experience porting from the v0 could be better?