Skip to content

fix: handle proprely document json in fast api response#1004

Merged
samsja merged 18 commits intofeat-rewrite-v2from
fix-fastapi-bug
Jan 10, 2023
Merged

fix: handle proprely document json in fast api response#1004
samsja merged 18 commits intofeat-rewrite-v2from
fix-fastapi-bug

Conversation

@samsja
Copy link
Copy Markdown
Member

@samsja samsja commented Jan 10, 2023 •
edited
Loading

Context

this PR fix several bug related to docarray, orjson and FastAPI

this is now working and return the proper tensor

     from docarray.document import DocumenResponse

    @app.post("/doc/", response_model=OutputDoc, response_class=DocumentResponse)
    async def create_item(doc: InputDoc) -> OutputDoc:
        ## call my fancy model to generate the embeddings
        doc = OutputDoc(
            embedding_clip=np.zeros((100, 1)), embedding_bert=np.zeros((100, 1))
        )
        return doc

samsja added 10 commits January 10, 2023 16:07
@samsja
test: add failing test for fastapi
1111581 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
refactor: renamed to json compatible and put it at abstract class level
d45377f 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
refactor: use fixture in tests to json
acddcd7 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
fix(tests): uses orjson response
2abc3d1 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
refactor: rename orjson dump
b62e2b8 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
feat: add document response
0642f58 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
refactor: better test
91f2c0b 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
fix: starlette not needed
5ed0828 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
fix: better fastapi support
274dd29 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja
fix: tests fastapi
ae39021 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja samsja changed the title Fix fastapi bug fix: handle proprely document json in fast api response Jan 10, 2023
@samsja
fix: apply black
3b84ddd 
Signed-off-by: Sami Jaghouar <[email protected]>
JohannesMessner
JohannesMessner reviewed Jan 10, 2023
View reviewed changes
@samsja
fix: fix import wo fast api
facbee2 
Signed-off-by: Sami Jaghouar <[email protected]>
JohannesMessner
JohannesMessner requested changes Jan 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@JohannesMessner JohannesMessner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have to rebase with the new file structure with docarray.documents etc

JohannesMessner
JohannesMessner approved these changes Jan 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@JohannesMessner JohannesMessner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already approving so we don't have to go back and forth 1M times

@samsja
fix: update docstring
ecf2992 
Signed-off-by: Sami Jaghouar <[email protected]>
@samsja samsja merged commit 56434bf into feat-rewrite-v2 Jan 10, 2023
@samsja samsja deleted the fix-fastapi-bug branch January 10, 2023 16:37
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 10, 2023

📝 Docs are deployed on https://ft-fix-fastapi-bug--jina-docs.netlify.app 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JohannesMessner JohannesMessner JohannesMessner approved these changes

+1 more reviewer

@anna-charlotte anna-charlotte anna-charlotte left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/core area/setup area/testing area/typing size/m

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@samsja @JohannesMessner @anna-charlotte