gmc: include ui as part of pipeline #435
Conversation
|
@irisdingbj @zhlsunshine I create a draft PR to include UI as part of the GMC pipeline. Current logic is based on what i discussed with @zhlsunshine before, please help with comments and review. Simple description: |
There was a problem hiding this comment.
Hi @Ruoyu-y, thanks for your PR to add this feature to GMC, BTW, do you want to add the e2e test for the new example?
|
|
||
| // accessing UI result in access to assets under /assets uri | ||
| // create a handler to redirect that request to ui endpoint | ||
| func mcAssetHandler(w http.ResponseWriter, req *http.Request) { |
There was a problem hiding this comment.
Hi @Ruoyu-y I suggest that we should handle the situation if the UI step does not exists, then the user can know what happen.
There was a problem hiding this comment.
Sure. Let me add that.
| @@ -45,6 +47,7 @@ const ( | |||
| ServiceNode = "node" | |||
| DataPrep = "DataPrep" | |||
| Parameters = "parameters" | |||
| UI = "UI" | |||
There was a problem hiding this comment.
Hi @Ruoyu-y, I think this UI component is not the GMC UI, it's just for ChatQnA example, if so, how to re-name it to ChatQnAUI ?
There was a problem hiding this comment.
Hi @Ruoyu-y, I think this UI component is not the GMC UI, it's just for
ChatQnAexample, if so, how to re-name it toChatQnAUI?
This UI does not ONLY handle the case for ChatQnA, i also tried it with CodeGen. It can work too. From function perspective, i would think this is a UI config or handling logic for application serving. And i understand that the GMC UI you mentioned is for pipeline composition purpose, correct?
| @@ -71,6 +71,7 @@ const ( | |||
| SpeechT5Gaudi = "SpeechT5Gaudi" | |||
| Whisper = "Whisper" | |||
| WhisperGaudi = "WhisperGaudi" | |||
| UI = "UI" | |||
There was a problem hiding this comment.
Hi @Ruoyu-y, the same comment as above.
| // set the corresponding keyword for input data | ||
| var key string | ||
| switch nextHop.StepName { | ||
| case "Embedding": |
There was a problem hiding this comment.
Hi @Ruoyu-y, I think there should be only 2 cases: request from the mega service and directly to embedding service. Am I right?
There was a problem hiding this comment.
Let me expain, since GMC wants to pass through the payload directly to the pipeline. The format of the payload must fit the requirement of the pipeline's first component. I found that in the examples, the first hop of the pipeline is either embedding or llm. So here, i am trying to find the first hop of the pipeline and use the corresponding keyword to format the payload.
I am not sure why there would be request from the mega service in the GMC case, can you explain?
Ruoyu-y
force-pushed
the
ui_in_pipeline
branch
4 times, most recently
from
e8b9139
to
d748bd4
Compare
|
@Ruoyu-y please rebase and it can be merged |
* add ui handler in GMC router logic * add sample config file for chatqna with conversation ui Signed-off-by: Ruoyu Ying <[email protected]>
Ruoyu-y
force-pushed
the
ui_in_pipeline
branch
from
d748bd4
to
0612d5c
Compare
|
@Ruoyu-y can you remove draft and then we can get this PR in? |
* add ui handler in GMC router logic * add sample config file for chatqna with conversation ui Signed-off-by: Ruoyu Ying <[email protected]> Co-authored-by: Iris <[email protected]> (cherry picked from commit 2e06fda)
Description
Issues
n/a.Type of change
List the type of change like below. Please delete options that are not relevant.
Dependencies
List the newly introduced 3rd party dependency if exists.
Tests
Describe the tests that you ran to verify your changes.