piccolo_api
piccolo_api copied to clipboard
piccolo-orm
CRUD Endpoint POST response is a list
On making a POST request to a CRUD endpoint, the response is something like:
[{'id': 4}]
IMHO this should be a dict following good REST practices and not a list since we cannot post multiple objects to the CRUD Endpoint. The client should be able to run something like:
>>> response = requests.post(crud_endpoint_url, json=data).json()
>>> response['id']
# created object id
The problem stems from this line: https://github.com/piccolo-orm/piccolo_api/blob/master/piccolo_api/crud/endpoints.py#L819
Actually row.save().run() seems to always return a list with one object ... assuming this is the case, we could strip the object out of the list and simply return the dict:
response = await row.save().run()
item = response[0]
json = dump_json(item)
return CustomJSONResponse(json, status_code=201)
Also it returns 201 instead of 200 as in the OpenAPI schema (for FastAPIWrapper), the schema should be fixed or if it can't, change to 200
@mastercoms I think HTTP 201 CREATED is a good response status code because the request was successful and led to the creation of a new resource. Sorry if I'm wrong.
I think what's going on is we're returning a 201 from PiccoloCRUD, but FastAPIWrapper has the default of 200. They should be consistent really - it should be a quick fix.
https://github.com/piccolo-orm/piccolo_api/blob/40d431693602da33460d10b4b2e7257065f8bcf6/piccolo_api/fastapi/endpoints.py#L393
@dantownsend I don't think we should change anything because both PiccoloCRUD and FastAPIWrapper ,in my case, return correct 201 on POST request? Sorry if I don't understand correctly.
@sinisaos Sorry, I wasn't clear - they both return 201, but in the Swagger docs it probably say 200.
It's because we don't tell FastAPI what status code we're returning.
@dantownsend Aha, now I understand! Here we can add status_code=status.HTTP_201_CREATED, to add_api_route of POST method and it works.
Should we do it for all methods?
Should we do it for all methods?
@sinisaos yeah, at least for any which don't return 200
@dantownsend If you OK with this changes
fastapi_app.add_api_route(
path=root_url,
endpoint=post,
response_model=self.ModelOut,
status_code=status.HTTP_201_CREATED, # new line
methods=["POST"],
**fastapi_kwargs.get_kwargs("post"),
)
I can try to do this tomorrow for all the methods to have the same status codes in the Swagger docs as in the PiccoloCRUD.