-
Notifications
You must be signed in to change notification settings - Fork 6
add possibility to add a custom headers to calls to LLMs #120
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds the ability to inject custom HTTP headers into LLM API calls via a new environment variable AI_API_CUSTOM_HEADER. The implementation removes the hardcoded COPILOT_INTEGRATION_ID header and introduces a flexible mechanism for adding custom headers.
Key Changes:
- Added
get_custom_header()function to parse custom headers from theAI_API_CUSTOM_HEADERenvironment variable - Removed hardcoded
Copilot-Integration-Idheader from bothlist_capi_models()andTaskAgent.__init__() - Modified header construction to merge custom headers with standard headers using the dictionary merge operator
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/seclab_taskflow_agent/capi.py | Added get_custom_header() function for parsing environment-based custom headers; removed COPILOT_INTEGRATION_ID constant; updated list_capi_models() to use merged headers with custom header support |
| src/seclab_taskflow_agent/agent.py | Updated imports to include get_custom_header instead of COPILOT_INTEGRATION_ID; modified TaskAgent.__init__() to use custom headers for AsyncOpenAI client initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parts = custom_header.split(':', 1) | ||
| if len(parts) != 2: | ||
| logging.warning(f"Invalid AI_API_CUSTOM_HEADER format. Expected 'name:value', got: {custom_header}") | ||
| return {} | ||
|
|
||
| name, value = parts | ||
| name = name.strip() | ||
| value = value.strip() | ||
| if not name or not value: | ||
| logging.warning(f"Invalid AI_API_CUSTOM_HEADER: header name and value must be non-empty after stripping. Got: '{custom_header}'") | ||
| return {} | ||
| return {name: value} |
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.
| parts = custom_header.split(':', 1) | |
| if len(parts) != 2: | |
| logging.warning(f"Invalid AI_API_CUSTOM_HEADER format. Expected 'name:value', got: {custom_header}") | |
| return {} | |
| name, value = parts | |
| name = name.strip() | |
| value = value.strip() | |
| if not name or not value: | |
| logging.warning(f"Invalid AI_API_CUSTOM_HEADER: header name and value must be non-empty after stripping. Got: '{custom_header}'") | |
| return {} | |
| return {name: value} | |
| return json.loads(custom_header) |
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.
Is there a simplified version of JSON supported by the json.loads API?
AI_API_CUSTOM_HEADER={\"key\":\"val\"} seems like a complication of things.
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.
Not that I know of. I agree that it makes the syntax more clunky. But it simplifies the code a lot and adds support for multiple fields, which somebody might find useful. This isn't going to be a frequently used feature is it?
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.
It is actually needed all the time after removing the default integration id, so I'd rather not having to do something like AI_API_CUSTOM_HEADER={\"key\":\"val\"} all the time. Besides nobody will use it for multiple header if they need to add that all those dashes everywhere.
No description provided.