Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrating gradio demo #6034

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ankur-singh
Copy link

@Ankur-singh Ankur-singh commented Aug 27, 2023

Goals:

resolves #6026

  • Add basic static gradio App
  • Make the app dynamic i.e. update the input and output blocks based on input and output model
  • call executor to close the loop (input -> executor -> output)

@Ankur-singh
Copy link
Author

I have added the basic code but for some reason gradio app is not working. I will give it another try in a day or two.

Another issue that I can use some help is, how to get hold of executor directly w/o creating the request and response objects. Once I have the input from the gradio app, I want a way to directly call the executor's main method.

@@ -36,6 +36,7 @@ docarray>=0.16.4: core
jina-hubble-sdk>=0.30.4: core
jcloud>=0.0.35: core
opentelemetry-api>=1.12.0: core
gradio: core
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if it is good to add this dependency as a core dependency. How big and how many extra dependencies does gradio bring?

Copy link
Author

Choose a reason for hiding this comment

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

yes, makes sense. Do you want me to add it some other type or remove it all together (and ask users to install it separately) ?

Copy link
Member

Choose a reason for hiding this comment

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

For now lets ask users to install it, let think about it later then

@@ -1,5 +1,7 @@
import inspect
from typing import TYPE_CHECKING, Callable, Dict, List, Optional, Union
import gradio as gr
Copy link
Member

Choose a reason for hiding this comment

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

this would imply thay impprting gradio should be protected, and the feature should be activated via an argument for Deployment or Flow

Copy link
Author

Choose a reason for hiding this comment

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

got it, I will move it inside the function call or if block

@@ -222,3 +233,62 @@ def _doc_to_event(doc):
return {'event': 'update', 'data': doc.to_dict()}
else:
return {'event': 'update', 'data': doc.dict()}

def generate_gradio_interface(model: BaseModel):
Copy link
Member

Choose a reason for hiding this comment

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

I think these have to be more ambitious and also cover docarray built-in types.

Copy link
Author

Choose a reason for hiding this comment

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

I read that docarray is compatible with pydantic. Any thing that is different and you want me to pay special attention to?

Copy link
Member

Choose a reason for hiding this comment

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

There are some special prebuilt types such as TextDoc, ImageDoc, etc ...

@Ankur-singh Ankur-singh changed the title added initial code for gradio demo Integrating gradio demo Aug 28, 2023
@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Merging #6034 (96b81ea) into master (23c7b8c) will decrease coverage by 3.59%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #6034      +/-   ##
==========================================
- Coverage   77.58%   74.00%   -3.59%     
==========================================
  Files         144      144              
  Lines       13788    13827      +39     
==========================================
- Hits        10697    10232     -465     
- Misses       3091     3595     +504     
Flag Coverage Δ
jina 74.00% <0.00%> (-3.59%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
jina/serve/runtimes/worker/http_fastapi_app.py 0.00% <0.00%> (-82.15%) ⬇️

... and 25 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrated Gradio Demo App
2 participants