-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
feat: add custom HTTP transport with headers for OpenAI client #245
Conversation
WalkthroughThis update modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant LLM as LLM Initialization
participant HT as headerTransport
participant API as OpenAI API
App->>LLM: Call createLLM/createVisionLLM
LLM->>HT: Instantiate headerTransport
HT->>API: Send HTTP request with header "X-Title: paperless-gpt"
API-->>HT: Return API response
HT-->>LLM: Pass response to LLM
LLM-->>App: Return initialized LLM
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
main.go (2)
643-654
: Reduce code duplication in transport initialization.The custom transport initialization is duplicated between
createLLM
andcreateVisionLLM
. Consider extracting this into a helper function.+func createCustomHTTPClient() *http.Client { + customTransport := &headerTransport{ + transport: http.DefaultTransport, + headers: map[string]string{ + "X-Title": "paperless-gpt", + }, + } + httpClient := http.DefaultClient + httpClient.Transport = customTransport + return httpClient +} func createLLM() (llms.Model, error) { switch strings.ToLower(llmProvider) { case "openai": if openaiAPIKey == "" { return nil, fmt.Errorf("OpenAI API key is not set") } - customTransport := &headerTransport{ - transport: http.DefaultTransport, - headers: map[string]string{ - "X-Title": "paperless-gpt", - }, - } - httpClient := http.DefaultClient - httpClient.Transport = customTransport + httpClient := createCustomHTTPClient() return openai.New( openai.WithModel(llmModel), openai.WithToken(openaiAPIKey), openai.WithHTTPClient(httpClient), )Also applies to: 681-692
647-647
: Consider making the header value configurable.The "X-Title" header value is hardcoded. Consider making it configurable through environment variables for flexibility.
+// Add to global variables +var paperlessGptHeader = os.Getenv("PAPERLESS_GPT_HEADER") + +// Add to validateOrDefaultEnvVars() +if paperlessGptHeader == "" { + paperlessGptHeader = "paperless-gpt" +} // In createCustomHTTPClient headers: map[string]string{ - "X-Title": "paperless-gpt", + "X-Title": paperlessGptHeader, },Also applies to: 685-685
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main.go
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
main.go (1)
713-725
: LGTM! Clean implementation of the custom transport.The
headerTransport
implementation correctly follows thehttp.RoundTripper
interface pattern and safely adds headers to requests.
f5bd2fd
to
23632f6
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
main.go (2)
713-725
: Add nil transport handling and improve documentation.The
headerTransport
implementation is clean but could benefit from some improvements:// headerTransport is a custom http.RoundTripper that adds custom headers to requests type headerTransport struct { - transport http.RoundTripper + transport http.RoundTripper // underlying transport, defaults to http.DefaultTransport if nil headers map[string]string } // RoundTrip implements the http.RoundTripper interface func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if t.transport == nil { + t.transport = http.DefaultTransport + } for key, value := range t.headers { req.Header.Add(key, value) } return t.transport.RoundTrip(req) }
646-648
: Make headers configurable via environment variables.Consider making the headers configurable to improve flexibility:
+// Environment Variables +var ( + // ... existing variables ... + customHeaders = map[string]string{ + "X-Title": getEnvOrDefault("OPENAI_CLIENT_TITLE", "paperless-gpt"), + } +) // headerTransport is a custom http.RoundTripper that adds custom headers to requests type headerTransport struct { transport http.RoundTripper - headers: map[string]string{ - "X-Title": "paperless-gpt", - }, + headers: customHeaders, } +// getEnvOrDefault returns environment variable value or default if not set +func getEnvOrDefault(key, defaultValue string) string { + if value := os.Getenv(key); value != "" { + return value + } + return defaultValue +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
main.go
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
main.go (1)
643-725
: Implementation looks good overall!The custom HTTP transport implementation successfully adds headers to OpenAI API requests. The code is clean and follows Go best practices.
main.go
Outdated
// Create custom transport that adds headers | ||
customTransport := &headerTransport{ | ||
transport: http.DefaultTransport, | ||
headers: map[string]string{ | ||
"X-Title": "paperless-gpt", | ||
}, | ||
} | ||
|
||
// Create custom client with the transport | ||
httpClient := http.DefaultClient | ||
httpClient.Transport = customTransport | ||
|
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.
🛠️ Refactor suggestion
Reduce code duplication in transport creation.
The custom transport creation is duplicated between createLLM
and createVisionLLM
. Consider extracting this into a helper function:
+// createCustomHTTPClient returns an http.Client with custom headers
+func createCustomHTTPClient() *http.Client {
+ customTransport := &headerTransport{
+ transport: http.DefaultTransport,
+ headers: map[string]string{
+ "X-Title": "paperless-gpt",
+ },
+ }
+ client := http.DefaultClient
+ client.Transport = customTransport
+ return client
+}
func createLLM() (llms.Model, error) {
switch strings.ToLower(llmProvider) {
case "openai":
if openaiAPIKey == "" {
return nil, fmt.Errorf("OpenAI API key is not set")
}
- // Create custom transport that adds headers
- customTransport := &headerTransport{
- transport: http.DefaultTransport,
- headers: map[string]string{
- "X-Title": "paperless-gpt",
- },
- }
- // Create custom client with the transport
- httpClient := http.DefaultClient
- httpClient.Transport = customTransport
+ httpClient := createCustomHTTPClient()
Also applies to: 681-692
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
main.go (1)
705-717
: Consider adding error handling for the transport.The implementation is correct, but it could be enhanced with error handling for the underlying transport.
Apply this diff to add error handling:
func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if t.transport == nil { + return nil, fmt.Errorf("transport cannot be nil") + } for key, value := range t.headers { req.Header.Add(key, value) } return t.transport.RoundTrip(req) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
main.go
(4 hunks)main_test.go
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-arm64
- GitHub Check: build-amd64
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
main_test.go (1)
180-199
: LGTM! The test verifies the custom HTTP client functionality.The test correctly verifies that the custom HTTP client adds the expected header to requests.
main.go (3)
642-647
: LGTM! The OpenAI client is correctly configured with the custom HTTP client.The implementation properly integrates the custom HTTP client with the OpenAI client configuration.
668-673
: LGTM! The Vision OpenAI client is correctly configured with the custom HTTP client.The implementation properly integrates the custom HTTP client with the Vision OpenAI client configuration.
689-703
: LGTM! The custom HTTP client creation is well-implemented.The function follows the suggestion from past review comments to extract the transport creation logic into a helper function. The implementation is clean and reusable.
Closes #237
Summary by CodeRabbit
New Features
Bug Fixes