|
| 1 | +# Core class for service integration |
| 2 | + |
| 3 | +Status: In-Progress. |
| 4 | + |
| 5 | + |
| 6 | +## Background |
| 7 | + |
| 8 | +The integration with AI Core is based on multiple layers of API. |
| 9 | +Depending on the usage scenario the user wants to interact with (1) AI Core, (2) foundation models, or (3) the orchestration service. |
| 10 | +API design and expected behavior heavily depend on the use case and functional- and non-functional requirements. |
| 11 | +When evaluating use cases we have to consider _nice_ users following best-practices with the same priority as _naughty_ users who do not follow recommendations nor read JavaDoc. |
| 12 | + |
| 13 | +Our product is the API. |
| 14 | + |
| 15 | +---- |
| 16 | + |
| 17 | +## API design requirements |
| 18 | + |
| 19 | +Let's distinguish between functional and non-functional requirements. |
| 20 | + |
| 21 | +### Non-Functional requirements |
| 22 | + |
| 23 | +* **Stability** |
| 24 | + * The API should be stable upon release. |
| 25 | + * Additions and extensions are allowed. |
| 26 | + * Breaking changes to existing methods are not allowed. |
| 27 | +* **Consistency**: |
| 28 | + * The API should be reasonable consistent across all available service integrations. |
| 29 | + * This includes naming conventions, method signatures, and compatible return types. |
| 30 | +* **Usability**: |
| 31 | + * The API should be easy to use and understand. |
| 32 | + * The user is expected to mostly auto-complete the code. |
| 33 | + Therefore fluent API design is expected. |
| 34 | + * The user expects that all available methods may lead to a reasonable outcome. |
| 35 | + * The user expects logic to be customizable. |
| 36 | + * General code guidelines apply like JavaDoc, method names, and parameter count and -names. |
| 37 | +* **Simplicity**: |
| 38 | + * The core API should not (immediately) expose any internal methods or classes. |
| 39 | + * No ambiguity. |
| 40 | + No repetition and redundant data. |
| 41 | + * Low hierarchy of API, i.e. deep nesting of classes is not required. |
| 42 | + * Object immutability is preferred. |
| 43 | +* **Transparency**: |
| 44 | + * The API should be transparent about its behavior. |
| 45 | + * Use correct method name prefix to indicate actions, e.g. `get`, `list`, `resolve`, `update`. |
| 46 | + |
| 47 | + |
| 48 | +SAP Cloud SDK flavored requirements: |
| 49 | +* Consider public interfaces over specific classes to be exposed to user. |
| 50 | + This makes implementations easier to extend and replace in the future. |
| 51 | + |
| 52 | +### Functional requirements |
| 53 | + |
| 54 | +* **Optimization** |
| 55 | + * Avoid redundant operations that cost heavy resources like time or memory, e.g. HTTP requests. |
| 56 | + * Avoid premature computation of data. |
| 57 | + * When evaluating, consider multi-threading and parallelism. |
| 58 | + For shared objects, consider thread-safety. |
| 59 | +* **Documentation** |
| 60 | + * Document public API usage in our repository. |
| 61 | + * Document best-practices and recommended API usage. |
| 62 | + * Document pitfalls and bad-practices. |
| 63 | + * Document error messages and solutions. |
| 64 | + |
| 65 | +SAP Cloud SDK flavored requirements: |
| 66 | +* Log activity, intermediate results, fallbacks and errors. |
| 67 | + |
| 68 | + |
| 69 | +## State of Core class |
| 70 | + |
| 71 | +| API | Behavior | |
| 72 | +|--------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------| |
| 73 | +| `new AiCoreService()` | <ul><li>☐ Resolve Service Binding / Destination</li><li>☐ Resolve Deployments (cached)</li><li>☐ Instantiate Cache</li></ul> | |
| 74 | +| `withDestination()` | <ul><li>☐ Resolve Service Binding / Destination</li><li>☐ Resolve Deployments (cached)</li><li>☐ Instantiate Cache</li></ul> | |
| 75 | +| `forDeployment()`<br> `forDeploymentByModel()`<br> `forDeploymentbyScenario()` | <ul> <li>☐ Resolve Service Binding / Destination</li> <li>☐ Resolve Deployments (cached)</li> <li>☒ Instantiate Cache</li> </ul> | |
| 76 | +| `withResourceGroup()` | <ul> <li>☐ Resolve Service Binding / Destination</li> <li>☐ Resolve Deployments (cached)</li> <li>☒ Instantiate Cache</li> </ul> | |
| 77 | +| `client()` | <ul> <li>☒ Resolve Service Binding / Destination</li> <li>☒ Resolve Deployments (cached)</li> <li>☐ Instantiate Cache</li> </ul> | |
| 78 | +| `destination()` | <ul> <li>☒ Resolve Service Binding / Destination</li> <li>☒ Resolve Deployments (cached)</li> <li>☐ Instantiate Cache</li> </ul> | |
| 79 | + |
| 80 | +Properties: |
| 81 | +* Lazy evaluation of deployments |
| 82 | +* Lazy evaluation of service binding (if no destination is provided) |
| 83 | +* Service Binding lookup itself is not cached by AI SDK itself, but by Service Binding library. |
| 84 | +* Immutable objects. |
| 85 | +* Custom method overloads on (sub-classes of) `AiCoreService` are propagated down to the client. |
| 86 | + |
| 87 | +Consideration: |
| 88 | +* Always assume worst-case: |
| 89 | + * User instantiates a constant `AiCoreService` in a static context. |
| 90 | + * Custom Service Binding may be supplied by 3nd party library. |
| 91 | + Race condition for resolving service bindings may lead to inconsistent behavior. |
| 92 | + * User switches resource group at runtime. |
| 93 | + * User switches destination at runtime. |
| 94 | + * User switches deployment at runtime. |
| 95 | + |
| 96 | +Pro: |
| 97 | +- No redundant HTTP traffic. |
| 98 | +- Slim API contract, not many public methods. |
| 99 | + |
| 100 | +Con: |
| 101 | +- Allowing for Resource Group declaration **only** on deployment level requires API separation. |
| 102 | + Two classes are necessary, where one class would've been better. |
| 103 | +- High maintainability cost. |
| 104 | + Extending or changing the API and its inherent behavior will likely require a complete implementation rewrite. |
| 105 | + |
| 106 | +## Decision |
| 107 | + |
| 108 | +- postponed for now |
0 commit comments