feat: Add external plugins support - DO NOT MERGE#183
feat: Add external plugins support - DO NOT MERGE#183mayabar wants to merge 1 commit intollm-d:mainfrom
Conversation
Signed-off-by: Maya Barnea <mayab@il.ibm.com>
There was a problem hiding this comment.
I did not review beyond the first or second file.
IMO, this PR might be too big to review in one go and could benefit from breaking up (e.g.,):
- design doc/proposal (which should be approved separately)
- API definitions (e.g., JSON between client and server)
- sample server (e.g., python) and client, for a fixed function.
- improved client (e.g., configuration of http.Client for timeout handling, etc.)
There is no need to support all plugin types at once. Furthermore, since configuration via file is forthcoming, I would delay writing configuration code until it is merged. Otherwise it's throwaway code.
| github.com/prometheus/client_golang v1.22.0 | ||
| github.com/redis/go-redis/v9 v9.7.3 | ||
| github.com/stretchr/testify v1.10.0 | ||
| github.com/valyala/fasthttp v1.62.0 |
There was a problem hiding this comment.
q: what's the benefit of using a non standard HTTP implementation? If performance, then we should first prove that HTTP handling is a bottleneck.
|
|
||
| prefixScorerBlockSizeEnvKey = "PREFIX_SCORER_BLOCK_SIZE" | ||
| prefixScorerBlockSizeDefault = 256 | ||
|
|
There was a problem hiding this comment.
At this point the external process plugin is for demo only, so I would refrain from introducing and using all these environment variables (which would be eliminated by the config changes anyway).
There was a problem hiding this comment.
For a demo, you can just enable (or not) a fixed type plugins (e.g., filter or scorer) at a well known address (e.g., localhost:8000/8088 and run their container(s) in the same Pod)
| logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/util/logging" | ||
|
|
||
| "github.com/llm-d/llm-d-inference-scheduler/pkg/config" | ||
| externalhttp "github.com/llm-d/llm-d-inference-scheduler/pkg/scheduling/plugins/external/http" |
|
@elevran what’s the intention about this PR? |
|
It is not intended to be merged at this time and was created to share ideas with community and to solicit feedback (the concept of out of process plugins was presented at a community meeting and had corresponding code for those interested). |
|
one option would be to create a branch called |
|
Agree. |
Add initial version of server side support for external HTTP plugins.
Add sample filter plugin implemented in python
Fixes: #163
Fixes: #164
Fixes: #184