sent sdk version in headers #65
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds SDK version tracking by including a custom header in HTTP requests made by the Fishjam client. This enables server-side tracking of which SDK version is being used by clients.
Changes:
- Added import of
get_version()function to the OpenAPI client module - Added
x-sdk_versionheader with formatpy-{version}to both sync and async HTTP client methods inAuthenticatedClient
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._headers[self.auth_header_name] = ( | ||
| f"{self.prefix} {self.token}" if self.prefix else self.token | ||
| ) | ||
| self._headers["x-sdk_version"] = f"py-{get_version()}" |
There was a problem hiding this comment.
The header name uses an underscore in "x-sdk_version" which is inconsistent with HTTP header naming conventions. HTTP headers should use hyphens as separators. Change to "x-sdk-version" to follow standard conventions.
| self._headers["x-sdk_version"] = f"py-{get_version()}" | |
| self._headers["x-sdk-version"] = f"py-{get_version()}" |
| self._headers[self.auth_header_name] = ( | ||
| f"{self.prefix} {self.token}" if self.prefix else self.token | ||
| ) | ||
| self._headers["x-sdk_version"] = f"py-{get_version()}" |
There was a problem hiding this comment.
The header name uses an underscore in "x-sdk_version" which is inconsistent with HTTP header naming conventions. HTTP headers should use hyphens as separators. Change to "x-sdk-version" to follow standard conventions.
| self._headers[self.auth_header_name] = ( | ||
| f"{self.prefix} {self.token}" if self.prefix else self.token | ||
| ) | ||
| self._headers["x-sdk_version"] = f"py-{get_version()}" |
There was a problem hiding this comment.
The SDK version header is only added to AuthenticatedClient but not to the base Client class. This creates an inconsistency where only authenticated requests include the SDK version. For consistent tracking and analytics, the SDK version header should also be added in the Client.get_httpx_client() method (around line 91) and Client.get_async_httpx_client() method (around line 122).
| self._headers["x-sdk_version"] = f"py-{get_version()}" |
| self._headers[self.auth_header_name] = ( | ||
| f"{self.prefix} {self.token}" if self.prefix else self.token | ||
| ) | ||
| self._headers["x-sdk_version"] = f"py-{get_version()}" |
There was a problem hiding this comment.
The new SDK version header functionality lacks test coverage. Similar header functionality in the codebase (e.g., x-goog-api-client in test_gemini.py) has comprehensive tests. Consider adding tests that verify the SDK version header is correctly set in both sync and async HTTP clients.
Description
Describe your changes.
Motivation and Context
Why is this change required? What problem does it solve? If it fixes an open
issue, please link to the issue here.
Documentation impact
Types of changes
not work as expected)