-
Notifications
You must be signed in to change notification settings - Fork 13
feat: [Orchestration] Unit test embeddings and client refactoring #464
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,98 @@ | ||||
package com.sap.ai.sdk.orchestration; | ||||
|
||||
import static com.sap.ai.sdk.orchestration.OrchestrationJacksonConfiguration.getOrchestrationObjectMapper; | ||||
|
||||
import com.fasterxml.jackson.core.JsonProcessingException; | ||||
import com.fasterxml.jackson.databind.ObjectMapper; | ||||
import com.sap.ai.sdk.core.DeploymentResolutionException; | ||||
import com.sap.ai.sdk.core.common.ClientResponseHandler; | ||||
import com.sap.ai.sdk.core.common.ClientStreamingHandler; | ||||
import com.sap.cloud.sdk.cloudplatform.connectivity.ApacheHttpClient5Accessor; | ||||
import com.sap.cloud.sdk.cloudplatform.connectivity.HttpDestination; | ||||
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException; | ||||
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationNotFoundException; | ||||
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; | ||||
import java.io.IOException; | ||||
import java.util.function.Supplier; | ||||
import java.util.stream.Stream; | ||||
import javax.annotation.Nonnull; | ||||
import lombok.extern.slf4j.Slf4j; | ||||
import lombok.val; | ||||
import org.apache.hc.client5.http.classic.HttpClient; | ||||
import org.apache.hc.client5.http.classic.methods.HttpPost; | ||||
import org.apache.hc.core5.http.ContentType; | ||||
import org.apache.hc.core5.http.io.entity.StringEntity; | ||||
|
||||
@Slf4j | ||||
class OrchestrationHttpExecutor { | ||||
private final Supplier<HttpDestination> destinationSupplier; | ||||
private static final ObjectMapper JACKSON = getOrchestrationObjectMapper(); | ||||
|
||||
OrchestrationHttpExecutor(@Nonnull final Supplier<HttpDestination> destinationSupplier) | ||||
throws OrchestrationClientException { | ||||
this.destinationSupplier = destinationSupplier; | ||||
} | ||||
Comment on lines
+31
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Question) Why no lombok constructor? :) |
||||
|
||||
@Nonnull | ||||
<T> T execute( | ||||
@Nonnull final String path, | ||||
@Nonnull final Object payload, | ||||
@Nonnull final Class<T> responseType) { | ||||
try { | ||||
val json = JACKSON.writeValueAsString(payload); | ||||
log.debug("Serialized request into JSON payload: {}", json); | ||||
val request = new HttpPost(path); | ||||
request.setEntity(new StringEntity(json, ContentType.APPLICATION_JSON)); | ||||
|
||||
val client = getHttpClient(); | ||||
|
||||
val handler = | ||||
new ClientResponseHandler<>( | ||||
responseType, OrchestrationError.class, OrchestrationClientException::new) | ||||
.objectMapper(JACKSON); | ||||
return client.execute(request, handler); | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
} catch (JsonProcessingException e) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Question) Don't these need final? |
||||
throw new OrchestrationClientException("Failed to serialize request payload for " + path, e); | ||||
} catch (DeploymentResolutionException | ||||
| DestinationAccessException | ||||
| DestinationNotFoundException | ||||
| HttpClientInstantiationException | ||||
| IOException e) { | ||||
throw new OrchestrationClientException( | ||||
"Request to Orchestration service failed for " + path, e); | ||||
} | ||||
} | ||||
|
||||
@Nonnull | ||||
Stream<OrchestrationChatCompletionDelta> stream(@Nonnull final Object payload) { | ||||
try { | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
val json = JACKSON.writeValueAsString(payload); | ||||
val request = new HttpPost("/completion"); | ||||
request.setEntity(new StringEntity(json, ContentType.APPLICATION_JSON)); | ||||
val client = getHttpClient(); | ||||
|
||||
return new ClientStreamingHandler<>( | ||||
OrchestrationChatCompletionDelta.class, | ||||
OrchestrationError.class, | ||||
OrchestrationClientException::new) | ||||
.objectMapper(JACKSON) | ||||
.handleStreamingResponse(client.executeOpen(null, request, null)); | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
} catch (JsonProcessingException e) { | ||||
throw new OrchestrationClientException( | ||||
"Failed to serialize payload for streaming request", e); | ||||
} catch (IOException e) { | ||||
throw new OrchestrationClientException( | ||||
"Streaming request to the Orchestration service failed", e); | ||||
} | ||||
} | ||||
|
||||
@Nonnull | ||||
private HttpClient getHttpClient() { | ||||
val destination = destinationSupplier.get(); | ||||
log.debug("Using destination {} to connect to orchestration service", destination); | ||||
return ApacheHttpClient5Accessor.getHttpClient(destination); | ||||
} | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
/* | ||
* Internal Orchestration Service API | ||
* Orchestration is an inference service which provides common additional capabilities for business AI scenarios, such as content filtering and data masking. At the core of the service is the LLM module which allows for an easy, harmonized access to the language models of gen AI hub. The service is designed to be modular and extensible, allowing for the addition of new modules in the future. Each module can be configured independently and at runtime, allowing for a high degree of flexibility in the orchestration of AI services. | ||
* | ||
* | ||
* | ||
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). | ||
* https://openapi-generator.tech | ||
* Do not edit the class manually. | ||
*/ | ||
|
||
package com.sap.ai.sdk.orchestration.model; | ||
|
||
import java.math.BigDecimal; | ||
import java.util.List; | ||
import javax.annotation.Nonnull; | ||
|
||
/** Embedding */ | ||
public interface Embedding { | ||
/** Helper class to create a String that implements {@link Embedding}. */ | ||
record InnerString(@com.fasterxml.jackson.annotation.JsonValue @Nonnull String value) | ||
implements Embedding {} | ||
|
||
/** | ||
* Creator to enable deserialization of a String. | ||
* | ||
* @param val the value to use | ||
* @return a new instance of {@link InnerString}. | ||
*/ | ||
@com.fasterxml.jackson.annotation.JsonCreator | ||
@Nonnull | ||
static InnerString create(@Nonnull final String val) { | ||
return new InnerString(val); | ||
} | ||
|
||
/** Helper class to create a list of BigDecimal that implements {@link Embedding}. */ | ||
record InnerBigDecimals( | ||
@com.fasterxml.jackson.annotation.JsonValue @Nonnull List<BigDecimal> values) | ||
implements Embedding {} | ||
|
||
/** | ||
* Creator to enable deserialization of a list of BigDecimal. | ||
* | ||
* @param val the value to use | ||
* @return a new instance of {@link InnerBigDecimals}. | ||
*/ | ||
@com.fasterxml.jackson.annotation.JsonCreator | ||
@Nonnull | ||
static InnerBigDecimals create(@Nonnull final List<BigDecimal> val) { | ||
return new InnerBigDecimals(val); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Minor)
static constant for
/completion