Refactor AiCoreService class #201
Conversation
| @@ -19,99 +19,104 @@ | |||
| * scenario or for a model. | |||
| */ | |||
| @Slf4j | |||
| class DeploymentCache { | |||
| @RequiredArgsConstructor | |||
| class DeploymentResolver { | |||
There was a problem hiding this comment.
The cache is actually just the hashmap, the majority of this class is logic doing the resolution
| } catch (final OpenApiRequestException e) { | ||
| log.error("Failed to load deployments into cache", e); | ||
| } catch (final RuntimeException e) { | ||
| throw new DeploymentResolutionException( |
There was a problem hiding this comment.
Note that this now throws. Previously the subsequent NoSuchElementException would have an incomplete stack trace, missing this cause.
| */ | ||
| @Nonnull | ||
| public AiCoreDeployment withResourceGroup(@Nonnull final String resourceGroup) { | ||
| aiCoreService.resourceGroup = resourceGroup; |
There was a problem hiding this comment.
this was a modification from the outside of another object, invisible for users. This has been refactored to no longer be needed
|
|
||
| /** Container for an API client and destination. */ | ||
| @FunctionalInterface | ||
| public interface AiCoreDestination { |
There was a problem hiding this comment.
The name AiCoreDestination for me was a bit too close to the Destination, and after this refactoring this interface isn't really needed anymore. Together with the goal of reducing public API this was removed.
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class DeploymentResolverTest extends WireMockTestServer { |
There was a problem hiding this comment.
renamed from CacheTest and optimized, but the diff is too large for git to detect it as renaming
…-classes' into chore/refactor-core-classes
orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClient.java
Show resolved
Hide resolved
|
Added an alternative with a Builder: #211 |
Context
Follow up of #79
This PR simplifies the core classes and reduces public API.
This comes at the cost of no longer supporting a flow where e.g. a scenario for a deployment lookup is given first, and a resource group given afterwards. Both now always have to be given together. This way, we avoid complicated state changes via side effects and potentially unexpected behavior.
Feature scope:
AiCoreServiceclassAiCoreServiceis now immutableDeploymentResolverDeploymentResolverDefinition of Done
Aligned changes with the JavaScript SDKRelease notes updated