Skip to content

Commit 3bcb7da

Browse files
author
Huimin Tai
committed
fix: complete SAP AI Core provider implementation with tests and retry fixes
- Fix ensureToken/resolveDeploymentURL to return orchHTTPError for HTTP failures, making them recognizable by isRetryableError() for proper retry - Fix log message to reflect all 3 endpoints (not just completion) - Add comprehensive handleStream unit tests (10 test cases) - Add Python unit tests for KAgentSAPAICoreLlm (41 test cases) - Add warning log URL context in Python retry path Signed-off-by: Huimin Tai <huimin.tai@sap.com>
1 parent f126dbc commit 3bcb7da

File tree

9 files changed

+1985
-67
lines changed

9 files changed

+1985
-67
lines changed

go/adk/pkg/models/sapaicore.go

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package models
22

33
import (
4+
"context"
45
"encoding/json"
56
"fmt"
67
"net/http"
@@ -47,7 +48,7 @@ func NewSAPAICoreModelWithLogger(config SAPAICoreConfig, logger logr.Logger) (*S
4748
}, nil
4849
}
4950

50-
func (m *SAPAICoreModel) ensureToken() (string, error) {
51+
func (m *SAPAICoreModel) ensureToken(ctx context.Context) (string, error) {
5152
m.mu.Lock()
5253
defer m.mu.Unlock()
5354

@@ -66,18 +67,25 @@ func (m *SAPAICoreModel) ensureToken() (string, error) {
6667
tokenURL += "/oauth/token"
6768
}
6869

69-
resp, err := m.httpClient.PostForm(tokenURL, url.Values{
70+
formData := url.Values{
7071
"grant_type": {"client_credentials"},
7172
"client_id": {clientID},
7273
"client_secret": {clientSecret},
73-
})
74+
}
75+
req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(formData.Encode()))
76+
if err != nil {
77+
return "", fmt.Errorf("failed to create OAuth2 token request: %w", err)
78+
}
79+
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
80+
81+
resp, err := m.httpClient.Do(req)
7482
if err != nil {
7583
return "", fmt.Errorf("OAuth2 token request failed: %w", err)
7684
}
7785
defer resp.Body.Close()
7886

7987
if resp.StatusCode != http.StatusOK {
80-
return "", fmt.Errorf("OAuth2 token request returned %d", resp.StatusCode)
88+
return "", &orchHTTPError{StatusCode: resp.StatusCode, URL: tokenURL}
8189
}
8290

8391
var tokenResp struct {
@@ -104,7 +112,7 @@ func (m *SAPAICoreModel) invalidateToken() {
104112
m.tokenExpiresAt = time.Time{}
105113
}
106114

107-
func (m *SAPAICoreModel) resolveDeploymentURL() (string, error) {
115+
func (m *SAPAICoreModel) resolveDeploymentURL(ctx context.Context) (string, error) {
108116
m.mu.Lock()
109117
if m.deploymentURL != "" && time.Now().Before(m.deploymentURLAt.Add(time.Hour)) {
110118
u := m.deploymentURL
@@ -113,13 +121,13 @@ func (m *SAPAICoreModel) resolveDeploymentURL() (string, error) {
113121
}
114122
m.mu.Unlock()
115123

116-
token, err := m.ensureToken()
124+
token, err := m.ensureToken(ctx)
117125
if err != nil {
118126
return "", err
119127
}
120128

121129
reqURL := fmt.Sprintf("%s/v2/lm/deployments", m.Config.BaseUrl)
122-
req, err := http.NewRequest("GET", reqURL, nil)
130+
req, err := http.NewRequestWithContext(ctx, "GET", reqURL, nil)
123131
if err != nil {
124132
return "", err
125133
}
@@ -133,7 +141,7 @@ func (m *SAPAICoreModel) resolveDeploymentURL() (string, error) {
133141
defer resp.Body.Close()
134142

135143
if resp.StatusCode != http.StatusOK {
136-
return "", fmt.Errorf("list deployments returned %d", resp.StatusCode)
144+
return "", &orchHTTPError{StatusCode: resp.StatusCode, URL: reqURL}
137145
}
138146

139147
var result struct {

go/adk/pkg/models/sapaicore_adk.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"bytes"
66
"context"
77
"encoding/json"
8+
"errors"
89
"fmt"
910
"io"
1011
"iter"
@@ -27,7 +28,12 @@ func (m *SAPAICoreModel) GenerateContent(ctx context.Context, req *model.LLMRequ
2728
if isRetryableError(err) {
2829
m.invalidateToken()
2930
m.invalidateDeploymentURL()
30-
m.Logger.Info("SAP AI Core request failed, retrying", "error", err)
31+
var he *orchHTTPError
32+
if errors.As(err, &he) {
33+
m.Logger.Info("SAP AI Core request failed, retrying", "status", he.StatusCode, "url", he.URL)
34+
} else {
35+
m.Logger.Info("SAP AI Core request failed, retrying", "error", err)
36+
}
3137
resp, err = m.doRequest(ctx, req, stream)
3238
if err != nil {
3339
yield(nil, fmt.Errorf("SAP AI Core retry failed: %w", err))
@@ -49,12 +55,12 @@ func (m *SAPAICoreModel) GenerateContent(ctx context.Context, req *model.LLMRequ
4955
}
5056

5157
func (m *SAPAICoreModel) doRequest(ctx context.Context, req *model.LLMRequest, stream bool) (*http.Response, error) {
52-
deploymentURL, err := m.resolveDeploymentURL()
58+
deploymentURL, err := m.resolveDeploymentURL(ctx)
5359
if err != nil {
5460
return nil, err
5561
}
5662

57-
token, err := m.ensureToken()
63+
token, err := m.ensureToken(ctx)
5864
if err != nil {
5965
return nil, err
6066
}
@@ -82,7 +88,7 @@ func (m *SAPAICoreModel) doRequest(ctx context.Context, req *model.LLMRequest, s
8288
if resp.StatusCode != http.StatusOK {
8389
defer resp.Body.Close()
8490
errBody, _ := io.ReadAll(io.LimitReader(resp.Body, 1024))
85-
return nil, &orchHTTPError{StatusCode: resp.StatusCode, Body: string(errBody)}
91+
return nil, &orchHTTPError{StatusCode: resp.StatusCode, Body: string(errBody), URL: url}
8692
}
8793

8894
return resp, nil
@@ -91,10 +97,11 @@ func (m *SAPAICoreModel) doRequest(ctx context.Context, req *model.LLMRequest, s
9197
type orchHTTPError struct {
9298
StatusCode int
9399
Body string
100+
URL string
94101
}
95102

96103
func (e *orchHTTPError) Error() string {
97-
return fmt.Sprintf("SAP AI Core returned HTTP %d: %s", e.StatusCode, e.Body)
104+
return fmt.Sprintf("SAP AI Core returned HTTP %d (url: %s): %s", e.StatusCode, e.URL, e.Body)
98105
}
99106

100107
func isRetryableError(err error) bool {

0 commit comments

Comments
 (0)