Added deployment cache #53
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CharlesDuboisSAP
changed the title
# Conflicts: # core/src/test/java/com/sap/ai/sdk/core/WireMockTestServer.java # core/src/test/java/com/sap/ai/sdk/core/client/ScenarioUnitTest.java # foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClient.java # orchestration/src/test/java/com/sap/ai/sdk/orchestration/client/OrchestrationUnitTest.java # sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/ConfigurationController.java
# Conflicts: # core/src/main/java/com/sap/ai/sdk/core/AiCoreDeployment.java # core/src/main/java/com/sap/ai/sdk/core/AiCoreService.java
newtork
reviewed
Oct 15, 2024
# Conflicts: # core/src/main/java/com/sap/ai/sdk/core/AiCoreDeployment.java # core/src/main/java/com/sap/ai/sdk/core/AiCoreService.java
# Conflicts: # core/src/main/java/com/sap/ai/sdk/core/AiCoreDeployment.java
newtork
reviewed
Oct 16, 2024
| class Application { | ||
| public class Application { | ||
| /** The API client connected to the AI Core service. */ | ||
| public static final ApiClient API_CLIENT = new AiCoreService().client(); |
There was a problem hiding this comment.
(Minor)
I'm not a fan of this implementation/declaration as it's unusual for SpringBoot application development.
More common approach would be to define a @Bean here and @Autowired it when using in controller classes.
@Bean
public ApiClient getApiCoreClient() {
return new AiCoreService().client();
}But we can also do this in a separate PR. That'll be fine.
newtork
reviewed
Oct 16, 2024
newtork
previously approved these changes
Oct 16, 2024
# Conflicts: # core/src/main/java/com/sap/ai/sdk/core/AiCoreService.java # core/src/test/java/com/sap/ai/sdk/core/client/WireMockTestServer.java
newtork
approved these changes
Oct 17, 2024
CharlesDuboisSAP
added a commit
that referenced
this pull request
Oct 17, 2024
* intermediate * ready-to-discuss state * Fix merge; Format * Add javadoc * Fix PMD warnings * Initial change of draft * Initial API change * Fix merge conflicts * Update readme * Fix PMD * Formatting * Apply suggestions from code review Co-authored-by: Charles Dubois <[email protected]> * Defuse functions * Update core/src/main/java/com/sap/ai/sdk/core/AiCoreServiceWithDeployment.java Co-authored-by: Charles Dubois <[email protected]> * Make service binding accessor changeable, e.g. for future testing * Rename methods and types; Fix merge * Formatting * Fix code, tests are working * Fix header provisioning * Add tests; Move stuff around * make base class extensible * work in progress * refine work in progress * Fix tests * Update test * Add annotations * Format * Format; JavaDoc * Restructure code * Fix PMD * Fix PMD * Fix test * Fix test * Apply review comments * Apply review comments * Add assertion on AI-Client-Type header * Minor JavaDoc fix * Formatting * Added deployment cache (#53) * Added query filters for Orchestration and OpenAI deployments * Added deployment cache * Added cache reset * Added unit tests * Fixed unit tests * PMD * Refactor to fix tests * fix tests for real * PMD + added clearCache and loadCache * PMD again * Added query filters for Orchestration and OpenAI deployments * Added deployment cache * Added cache reset * Added unit tests * Fixed unit tests * PMD * Refactor to fix tests * fix tests for real * PMD + added clearCache and loadCache * PMD again * WiP * Delete Core * Formatting * Fixed tests * Single cache * Added clearCache test * Fix tests * Added resetCache * Merged * spotless * Reduced code * Added throws * Added API client to application startup * Make DeploymentCache package private and an instance * Merged main * Changed cache to a Set --------- Co-authored-by: SAP Cloud SDK Bot <[email protected]> --------- Co-authored-by: Alexander Dümont <[email protected]> Co-authored-by: SAP Cloud SDK Bot <[email protected]> Co-authored-by: Charles Dubois <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
AI/ai-sdk-java-backlog#54.
The lookup of deployments should be more efficient & resilient.
Note for reviewers:
The filter for Orchestration and OpenAI deployments has been removed since the new
AiCoreServiceallows to fetch all deploymentsFeature scope:
new AiCoreService().forDeployment...DeploymentCache.resetCache()whenever they delete a deploymentDefinition of Done
Documentation updatedRelease notes updated