foundation-model-stack / fms-guardrails-orchestrator

🚀 Guardrails orchestration server for application of various detections on text generation input and output.
https://foundation-model-stack.github.io/fms-guardrails-orchestrator/
Apache License 2.0
2 stars 15 forks source link

Change detector calls, so that only hostname is needed in detectors configuration #142

Open mdevino opened 2 months ago

mdevino commented 2 months ago

Description

As an orchestrator contributor, I want to change the code calling detector endpoints, so that I can set only the hostname in the hostname parameter in detectors configuration (e.g. https://my-detector.com instead of https://my-detector.com/api/v1/text/generation).

Note: This should be done for all detector endpoints.

Discussion

This was raised as an issue as a comment in one of the recent PRs. Making this change would allow two things:

  1. Support detectors that implement more than one endpoint (detectors API reference).

I've tried the following:

    /// Invokes detectors implemented with the `/api/v1/text/generation` endpoint
    pub async fn generation_detection(
        &self,
        model_id: &str,
        request: GenerationDetectionRequest,
    ) -> Result<Vec<DetectionResult>, Error> {
        let client = self.client(model_id)?;
-        let url = client.base_url().as_str();
+       let url = client.base_url().join("/api/v1/text/generation").unwrap();
        let response = client
-            .post(url)
+            .post(url.as_str())
            .header(DETECTOR_ID_HEADER_NAME, model_id)
            .json(&request)
            .send()
            .await?;
        if response.status() == StatusCode::OK {
            Ok(response.json().await?)
        } else {
            let error = response.json::<DetectorError>().await.unwrap();
            Err(error.into())
        }
    }

As a result, detector hostnames could be configured as desired. I also noticed that, if a detector was configured with a full url (say https://mydetector.com/a/full/url), the path was entirely replaced (i.e. https://mydetector.com/a/full/url becomes https://mydetector.com/api/v1/text/generation, not https://mydetector.com/a/full/url/api/v1/text/generation.

Acceptance Criteria

mdevino commented 6 days ago

This is going to be addressed by #205.

declark1 commented 6 days ago

This is required for the client refactor, so I am adding validation to ensure only base urls are set in the config for http clients.