Add detector type to config #205
Conversation
mdevino
requested review from
gkumbhat,
evaline-ju and
declark1
as code owners
mdevino
force-pushed
the
detector-type
branch
from
d620933 to
4e9400d
Compare
| @@ -129,16 +129,31 @@ impl From<tonic::Status> for Error { | |||
| pub struct HttpClient { | |||
| base_url: Url, | |||
| client: reqwest::Client, | |||
| detector_type: DetectorType, | |||
There was a problem hiding this comment.
Is this okay, or should we have a "subclass" for detector clients?
| pub fn endpoint(&self) -> String { | ||
| let path = match self.detector_type { | ||
| DetectorType::Content => "/api/v1/text/contents", | ||
| DetectorType::Generated => "/api/v1/text/generation", | ||
| DetectorType::Chat => "/api/v1/text/context/chat", | ||
| DetectorType::Context => "/api/v1/text/context/doc", | ||
| }; | ||
| self.base_url.join(path).unwrap().to_string() | ||
| } |
There was a problem hiding this comment.
Would it be better to return String, &'static str or &Url here?
| #[cfg_attr(test, derive(PartialEq))] | ||
| #[serde(rename_all = "lowercase")] | ||
| pub enum DetectorType { | ||
| #[default] |
There was a problem hiding this comment.
Note: even though this is set as default, "type" is mandatory on the YAML.
Signed-off-by: Mateus Devino <[email protected]>
Signed-off-by: Mateus Devino <[email protected]>
Signed-off-by: Mateus Devino <[email protected]>
Signed-off-by: Mateus Devino <[email protected]>
Signed-off-by: Mateus Devino <[email protected]>
Signed-off-by: Mateus Devino <[email protected]>
This reverts commit 96c5098. Signed-off-by: Mateus Devino <[email protected]>
Signed-off-by: Mateus Devino <[email protected]>
mdevino
force-pushed
the
detector-type
branch
from
8a817b9 to
61348f3
Compare
Signed-off-by: Mateus Devino <[email protected]>
There was a problem hiding this comment.
We should not add detector concepts, e.g. detector_type and endpoint with detector-specific endpoints to HttpClient. This is meant to be a reusable abstraction for any HTTP API, e.g. we will be using it for the upcoming OpenAI chat & chat completions implementation.
@declark1 Agreed, but not quite sure on how to go about this in a Rusty way. Should I move these out to a trait, or create a whole new struct? |
|
Closing this as we are consolidating client-related changes into #220 |
This addresses #165.
I'm opening this as I'd like to get some feedback on what I've done so far.
Missing tasks:
text_contents(),text_generation()andtext_context_doc()` - as faux has a limitation when dealing with generics, I've rolled back this change and will work on it in a different PR.