Feat/wrapper #90
Feat/wrapper #90
Conversation
dosubot
bot
added
the
size:XXL
|
Can you please check and remove comments from the PR |
|
Hey @csgulati09 - I can see that test directory for threads, assistants and images contains config with virtual keys from providers like cohere, anthropic and anyscale. Should we remove them as these providers do not support the methods |
tests/test_assistants.py
Outdated
| for i in get_configs(f"{CONFIGS_PATH}/single_provider"): | ||
| t3_params.append((client, i)) | ||
|
|
||
| @pytest.mark.parametrize("client, provider, auth, model", t3_params) |
There was a problem hiding this comment.
t3_params only has 2 items: client and virtual_key as per the above initialization. So @pytest.mark.parametrize("client, provider, auth, model", t3_params) will fail as it checks for 4
tests/test_threads.py
Outdated
| for i in get_configs(f"{CONFIGS_PATH}/single_with_basic_config"): | ||
| t2_params.append((client, i)) | ||
|
|
||
| @pytest.mark.parametrize("client, provider, auth, model", t2_params) |
There was a problem hiding this comment.
The same problem as assistants tests here as well. t2_params only has client and virtual_key but the parametrize expects 4.
| @@ -33,3 +33,4 @@ | |||
| PORTKEY_GATEWAY_URL = PORTKEY_BASE_URL | |||
| PORTKEY_API_KEY_ENV = "PORTKEY_API_KEY" | |||
| PORTKEY_PROXY_ENV = "PORTKEY_PROXY" | |||
| OPEN_AI_API_KEY = "DUMMY-KEY" | |||
There was a problem hiding this comment.
I think this is not required anymore, right? We can remove if thats the case.
There was a problem hiding this comment.
We need some value as a placeholder. Even it has no significance, but we need some string to pass.
setup.cfg
Outdated
| @@ -42,6 +42,7 @@ dev = | |||
| python-dotenv==1.0.0 | |||
| ruff==0.0.292 | |||
| pytest-asyncio==0.23.5 | |||
| openai>=1.12.0,<1.12.9 | |||
There was a problem hiding this comment.
We might have to allow all the 1.x.x versions here as openai is already on 1.13.0 version. Might have to test this as well by installing portkey to a project that already has openai installed in it.
| model=model, messages=messages, **kwargs | ||
| ) | ||
| json_response = json.loads(response.text) | ||
| return ChatCompletions(**json_response) |
There was a problem hiding this comment.
I think due to this, get_headers() is also not working because only json fields are getting passed to the model. Need to debug this further. But get_headers() is returning None. Please check this once.
Wrapper/suggestions
|
Closes #89 |
Title: Revamp: wrapper around OpenAI SDK
Description:
Motivation:
This will make it easier for us to integrate newly introduced routes from OpenAI
Related Issues:
#89