core
core copied to clipboard
[Refactor] Unify endpoints response / REST API v2
At the moment all the endpoints response are not uniform ( sometimes we give jsonSchema, sometimes a json with some field etc) and can create problem when the user uses the REST API instead of Language Specific clients, like the Typescript client.
I suggest the use of JSend as standard JSON response for all endpoints and change all HTTPExceptions responses, FastAPI provide the override of these exceptions with a simple function.
Fast API is a typed Backend framework, so I suggest also the use of an object call JSendResponse
that wraps the JSend standard.
ALL of this must be under this versioned prefix /api/v2
so we can delay the breaking changes!
P.S JSend is usually the simplest and have many variations (only have status and data where wraps a string or an object, status can be the HTTP status code instead of a string etc…) !
@enrichman Thx for the versioned prefix suggestion!
- full CRUD over documents and chunks
-
user_id
on each endpoint - authentication???
(Notes from dev meeting)
V2 REST API:
-
DEVELOPMENT
- namespace under
/v2/...
- deprecation warning on v1 endpoints
- delete v1 endpoints at Cat v2 release
- migrate tests directly to v2
- namespace under
-
NAMING STANDARD
- NO SLASH con redirect verso SLASH
- dash notation / da vedere con le classi
- setting vs settings ???
- better semantic ordering (/plugins/nome-plugin/settings)
-
ENDPOINT HIERARCHY
- Settings (TO BE DELETED)
- LLM
- Embedder
- Plugin
- Memory
- only episodic and declarative
- resources are collection and point
- POST chunk + metadata
- Conversation
- GET / DELETE / PUT convo history
- Rabbithole
- no edits
-
MEMORY CRUD (only collection, point and metadata)
- only identity filters!
{"source": "my.pdf"}
(no: <, >, !=, in [], contains)
- only identity filters!
-
ENDPOINT CUSTOM
- (hacky) manual input
- (top) hook:
- we pass cheshire_cat_api as router, and you can use include_router
- we pass the router obj already namespaced under
/custom
- (complex) decorator + add_api_route done by mad_hatter
-
HTTP STREAMING
- stays hanging / experimental
I would like to work on the issue :) Before proceeding, I have a proposal regarding the implementation that I'd like to get feedback on.
My idea is to create 2 APIRouter and for each endpoint function create 2 functions (one for each version). The differences is that in the v2 endpoint function wraps the result into a JSendResponse. For example:
class JSendResponse(BaseModel):
status:JSendStatusEnum
data: Union[Dict, None] = None
message:Union[str, None] = None
# Default router
router_v1 = APIRouter()
# Router v2
router_v2 = APIRouter()
def home() -> Dict:
with open("pyproject.toml", "rb") as f:
project_toml = tomli.load(f)["project"]
return {
"status": "We're all mad here, dear!",
"version": project_toml['version']
}
# server status
@router_v1.get("/")
async def home_v1() -> Dict:
"""Server status"""
return home()
@router_v2.get("/", response_model=JSendResponse, response_model_exclude_none=True)
async def home_v2():
"""Server status"""
return {
"status" : JSendStatusEnum.success,
"data" : home()
}
And then in main.py add the endpoints for v2.
This implementation keeps the behaviour of the current endpoints but requires the refactoring of all the endpoint functions. What do you think? I am open to suggestions if you think there is a better way to implement this.
@dave90 agree on the refactoring strategy, I don't know about the JSON send because from the research I'm doing it is not widespread and only adds overhad on directly inspecting the status code of the http response.
Also many are suggesting to have Pydantic types for all endpoints, I don't see the point of encapsulating them inside {"data": {}}
I assigned this issue to you, start from something easy and do small PRs ;) Refactoring the endpoints one route at a time is a good idea, so we can test properly I'm available for help and support anytime
Thanks!!!
Agrre with @pieroit, Data should be a Union of Classes (LLMSettings,EmbedderSettings etc... )!
Agrre with @pieroit, Data should be a Union of Classes (LLMSettings,EmbedderSettings etc... )!
My points are:
- the standard you guys mention does not seem supported and well spread
- there is no point in repeating the status code
- data as a key is not needed, it just makes the json deeper
Agrre with @pieroit, Data should be a Union of Classes (LLMSettings,EmbedderSettings etc... )!
My points are:
* the standard you guys mention does not seem supported and well spread * there is no point in repeating the status code * data as a key is not needed, it just makes the json deeper
- Jsend is the easiest one and most REST responses are similar or follow the same logic of Jsend(See this). We can change it to this but is very complex and unnecessary.
- Jsend don't require the status code in the body, the status param is a enum of 3 strings:
success
,error
,fail
. - Data key contains the data of the response. For the user prospective is much easier, He knows that each time the response is 2xx the Data key is where i can find the data (in case of LLM models, i will get the Json Schema)
(Notes from dev meeting)
V2 REST API:
* DEVELOPMENT * namespace under `/v2/...` * deprecation warning on v1 endpoints * delete v1 endpoints at Cat v2 release * migrate tests directly to v2 * NAMING STANDARD * NO SLASH con redirect verso SLASH * dash notation / da vedere con le classi * setting vs settings ??? * better semantic ordering (/plugins/nome-plugin/settings) * ENDPOINT HIERARCHY * Settings (TO BE DELETED) * LLM * Embedder * Plugin * Memory * only episodic and declarative * resources are collection and point * POST chunk + metadata * Conversation * GET / DELETE / PUT convo history * Rabbithole * no edits * MEMORY CRUD (only collection, point and metadata) * only identity filters! `{"source": "my.pdf"}` (no: <, >, !=, in [], contains) * ENDPOINT CUSTOM * (hacky) manual input * (top) hook: * we pass cheshire_cat_api as router, and you can use include_router * we pass the router obj already namespaced under `/custom` * (complex) decorator + add_api_route done by mad_hatter * HTTP STREAMING * stays hanging / experimental
- Rabbithole
- Endpoints must accept a json rappresenting the metadata