refactor assistant streaming and create OpenAI compliant base class #425
Conversation
ragna/assistants/_api.py
Outdated
There was a problem hiding this comment.
This file was renamed to _http_api.py but has enough changes for git to not recognize it as such.
|
|
||
|
|
||
| class HttpApiAssistant(Assistant): | ||
| _API_KEY_ENV_VAR: Optional[str] |
There was a problem hiding this comment.
The API key is now optional. See #375 for a discussion.
| @@ -21,8 +21,8 @@ def _make_system_content(self, sources: list[Source]) -> str: | |||
| ) | |||
| return instruction + "\n\n".join(source.content for source in sources) | |||
|
|
|||
| async def _call_api( | |||
There was a problem hiding this comment.
The def _call_api abstraction for def answer was just a remnant of an old implementation that I forgot to clean up earlier:
ragna/ragna/assistants/_api.py
Lines 32 to 38 in 84cf4f6
This PR removes it and all subclass simply implement def answer directly.
|
|
||
|
|
||
| class AnthropicApiAssistant(ApiAssistant): | ||
| class AnthropicAssistant(HttpApiAssistant): |
There was a problem hiding this comment.
Driveby rename to align it with other provider base classes.
There was a problem hiding this comment.
This is a demonstration how easy it is after this PR to add new OpenAI compliant assistants.
| yield cast(str, choice["delta"]["content"]) | ||
|
|
||
|
|
||
| class OpenaiAssistant(OpenaiCompliantHttpApiAssistant): |
There was a problem hiding this comment.
The public OpenAI API fits the new scheme nicely.
| @pytest.mark.parametrize( | ||
| "assistant", | ||
| [assistant for assistant in HTTP_API_ASSISTANTS if assistant._API_KEY_ENV_VAR], | ||
| ) |
|
|
||
| @property | ||
| def _url(self) -> str: | ||
| base_url = os.environ.get("RAGNA_LLAMAFILE_BASE_URL", "http://localhost:8080") |
There was a problem hiding this comment.
@nenb is port 8080 the default?
|
If this PR is accepted, I'll have a go at #376 and bring it up to speed. |
|
|
||
| @property | ||
| def _url(self) -> str: | ||
| base_url = os.environ.get("RAGNA_LLAMAFILE_BASE_URL", "http://localhost:8080") |
|
Touching on #424 (comment)
That is certainly up for debate. I think the most practical thing given the variety of cases here is: Any REST API is OpenAI compliant if it uses the same request and response schema as OpenAI. For practicality reasons, we allow the following deviations:
|
This came from an offline discussion with @nenb and supersedes #424. It also paves the way for #375.
The main two changes are
More details inline.