Skip to content

feat: add SDK identification headers to all HTTP requests#268

Merged
skylarmb merged 5 commits into
federated-sdk-release-candidatefrom
devin/1772003759-add-sdk-version-header
Feb 26, 2026
Merged

feat: add SDK identification headers to all HTTP requests#268
skylarmb merged 5 commits into
federated-sdk-release-candidatefrom
devin/1772003759-add-sdk-version-header

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Feb 25, 2026

feat: add SDK identification headers to all HTTP requests

Summary

Adds three identification headers to every HTTP request made by the Python SDK so the backend can identify which SDK, language, and version originated each call:

Header Value
hh-sdk-version SDK __version__ (e.g. 1.0.0rc16), falls back to "unknown"
hh-sdk-language python
hh-sdk-package honeyhive

Approach: A new get_default_headers() method on APIConfig centralizes the standard header set (Content-Type, Accept, Authorization, and the three new SDK headers). Both the code-generation Jinja2 templates (httpx.jinja2, apiconfig_pydantic_2.jinja2) and the generated output were updated, so the headers survive future make generate runs. The CLI and tracer OTLP exporter were also updated directly.

Updates since last revision

  • Added hh-sdk-language and hh-sdk-package headers alongside hh-sdk-version (per discussion about distinguishing Python honeyhive from TS honeyhive / @honeyhive/logger)
  • Added 9 unit tests in tests_v2/unit/test_api_config_headers.py covering all header keys and values
  • Simplified CHANGELOG to customer-facing language
  • All prior fixes still apply: code-gen templates, graceful _get_sdk_version() fallback, Bearer None fix

Review & Testing Checklist for Human

  • End-to-end header verification: Make a test API call against staging and inspect request headers on the backend to confirm all three hh-sdk-* headers appear with the expected values. CI doesn't exercise a real backend.
  • Bearer when no token: When access_token is None, the Authorization header becomes "Bearer " (empty string after Bearer). Confirm the backend handles this gracefully — this is pre-existing behavior that was "Bearer None" before this PR.
  • Template header_params path: The httpx.jinja2 template has a conditional {% if header_params|length > 0 %} block. No current endpoints use header params, so this path is untested. If a future OpenAPI spec adds header parameters, verify make generate still produces valid Python.

Notes

  • The OTLP exporter path (tracer spans) uses a separate _get_sdk_version() helper since those headers have a different structure (X-Project, X-Source) and don't go through APIConfig.
  • Requested by @skylarmb
  • Link to Devin run

Add a centralized get_default_headers() method to APIConfig that includes
a 'hh-sdk-version' header with the SDK version. This allows HTTP calls
from the python-sdk client to be identified on the backend.

Changes:
- Added get_default_headers() to APIConfig in api_config.py
- Updated all generated service files (sync + async) to use it
- Updated manual header construction in client.py export methods
- Updated CLI request headers
- Updated tracer OTLP exporter headers

Co-Authored-By: unknown <>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented Feb 25, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 25, 2026

Claude finished @devin-ai-integration[bot]'s task in 2m 12s —— View job


Reviewing PR #268

  • Gather context and read changed files
  • Review api_config.py changes (core logic)
  • Review generated service files
  • Review client.py, cli/main.py, and tracer changes
  • Post inline comments for specific issues
  • Post top-level review summary

Summary

Reviewed all 22 changed files. The centralization of headers into get_default_headers() is a clean improvement that removes ~490 lines of duplication.

Posted 3 inline comments covering:

  1. Bearer None risk when access_token is None (minor bug)
  2. Graceful degradation suggestion for the lazy __version__ import
  3. Nit on _get_sdk_version() being a thin wrapper

Posted top-level review covering:

  • Generated file durability risk (changes will be lost on next code gen)
  • No tests added for the new header
  • Confirmation of no backwards compatibility issues

@github-actions
Copy link
Copy Markdown
Contributor

📚 Documentation preview built — Download artifact

Review instructions & validation status

How to Review

  1. Download the artifact from the link above
  2. Extract the files
  3. Open index.html in your browser

Validation Status

  • API validation: ✅ Passed
  • Build process: ✅ Successful
  • Import tests: ✅ All imports working

Comment thread src/honeyhive/_generated/api_config.py Outdated
Comment thread src/honeyhive/tracer/instrumentation/initialization.py Outdated
Comment thread src/honeyhive/_generated/api_config.py
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 25, 2026

Code Review Summary

Overall: Clean, well-scoped change. The centralization of headers into get_default_headers() is a net positive — it removes ~490 lines of duplication and makes future header additions trivial. The approach is sound.

What looks good

  • Lazy import strategy is correct. __version__ is defined at the top of __init__.py (line 6, before any submodule imports), so the from honeyhive import __version__ inside get_default_headers() is safe from circular imports at call time.
  • Consistent coverage — all 18 generated service files, client.py, CLI, and tracer OTLP exporter are updated.
  • The PR description's review checklist is thorough and honest about risks.

Concerns

  1. Generated file durability (medium risk): All _generated/services/*.py files were hand-edited. If there's a code generation step (e.g., openapi-python-generator or similar), these changes will be silently overwritten on next regeneration. Recommendation: Update the generator template or add a post-generation hook to emit api_config.get_default_headers() instead of inline headers. Otherwise this is a maintenance time-bomb.

  2. No tests added: There are no unit tests for get_default_headers() or for verifying the hh-sdk-version header is present in requests. A simple test asserting the header key and value format would catch regressions — especially important given the generated file durability concern above.

  3. Backwards compatibility: This PR is strictly additive (new header, no signature changes). No breaking changes detected.

Documentation

No documentation updates needed — this is an internal header not exposed in any public API.

See inline comments for specific code-level feedback.

- Updated httpx.jinja2 and apiconfig_pydantic_2.jinja2 templates so
  the hh-sdk-version header survives code regeneration
- Added _get_sdk_version() with try/except fallback to 'unknown'
- Fixed Bearer None when access_token is None (uses empty string)
- Added CHANGELOG entry for the feature
- Added graceful degradation to cli/main.py and initialization.py

Co-Authored-By: unknown <>
@github-actions
Copy link
Copy Markdown
Contributor

📚 Documentation preview built — Download artifact

Review instructions & validation status

How to Review

  1. Download the artifact from the link above
  2. Extract the files
  3. Open index.html in your browser

Validation Status

  • API validation: ✅ Passed
  • Build process: ✅ Successful
  • Import tests: ✅ All imports working

Comment thread CHANGELOG.md Outdated
Comment thread scripts/templates/httpx.jinja2
Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

…eader

- Removed implementation details from CHANGELOG (customer-facing only)
- Added 7 unit tests covering get_default_headers() and _get_sdk_version()

Co-Authored-By: unknown <>
@github-actions
Copy link
Copy Markdown
Contributor

📚 Documentation preview built — Download artifact

Review instructions & validation status

How to Review

  1. Download the artifact from the link above
  2. Extract the files
  3. Open index.html in your browser

Validation Status

  • API validation: ✅ Passed
  • Build process: ✅ Successful
  • Import tests: ✅ All imports working

Updated the description for the 'hh-sdk-version' header to clarify its purpose.
@github-actions
Copy link
Copy Markdown
Contributor

📚 Documentation preview built — Download artifact

Review instructions & validation status

How to Review

  1. Download the artifact from the link above
  2. Extract the files
  3. Open index.html in your browser

Validation Status

  • API validation: ✅ Passed
  • Build process: ✅ Successful
  • Import tests: ✅ All imports working

@devin-ai-integration devin-ai-integration Bot changed the title feat: add hh-sdk-version header to all HTTP requests feat: add SDK identification headers to all HTTP requests Feb 25, 2026
@github-actions
Copy link
Copy Markdown
Contributor

📚 Documentation preview built — Download artifact

Review instructions & validation status

How to Review

  1. Download the artifact from the link above
  2. Extract the files
  3. Open index.html in your browser

Validation Status

  • API validation: ✅ Passed
  • Build process: ✅ Successful
  • Import tests: ✅ All imports working

@skylarmb
Copy link
Copy Markdown
Contributor

@claude approve with final review

@skylarmb skylarmb marked this pull request as ready for review February 25, 2026 19:28
@skylarmb skylarmb requested a review from a team February 25, 2026 19:29
@skylarmb
Copy link
Copy Markdown
Contributor

@claude review the final diff and whether feedback was addressed

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 25, 2026

Claude finished @skylarmb's task in 1m 42s —— View job


Reviewing PR #268 — Final diff and feedback resolution

  • Gather context: read the full diff against base branch
  • Check if previous review feedback was addressed
  • Review new test file
  • Post final review summary

Previous Feedback — Resolution Status

Feedback Status Details
Bearer None bugget_access_token() returning None produced "Bearer None" Addressed Fixed with self.get_access_token() or '' in api_config.py:68. Test added at test_api_config_headers.py:42.
Graceful degradation — wrap lazy __version__ import in try/except Addressed _get_sdk_version() in both api_config.py:75-82 and initialization.py:50-57 uses try/except Exception with "unknown" fallback. CLI at main.py:347-351 also has the same pattern.
No tests — header behavior was untested Addressed 9 tests added in tests_v2/unit/test_api_config_headers.py covering all header keys, values, version matching, and the Bearer None fix.
CHANGELOG too implementation-detailed Addressed Simplified to customer-facing language: lists the three hh-sdk-* headers without internal details.
Code-gen template durability — hand-edits to generated files would be lost on make generate Addressed Both apiconfig_pydantic_2.jinja2 and httpx.jinja2 templates were updated. Generated files match the template output, so make generate will preserve the changes.
_get_sdk_version() indirection nit — helper used once in tracer Acknowledged Kept as-is for consistency with api_config.py. Reasonable decision.
Add hh-sdk-language and hh-sdk-package headers Addressed All three headers present in api_config.py, initialization.py, and cli/main.py.

Final Diff Review

All feedback from both Claude and @skylarmb has been addressed. The final state of the code is clean:

  • api_config.py: get_default_headers() returns all 6 headers (Content-Type, Accept, Authorization, hh-sdk-version, hh-sdk-language, hh-sdk-package). _get_sdk_version() is a module-level helper with graceful fallback.
  • Templates: Both apiconfig_pydantic_2.jinja2 and httpx.jinja2 are in sync with the generated output. The httpx template uses headers = api_config.get_default_headers() cleanly on line 17.
  • Generated services (18 files): All use api_config.get_default_headers() — no inline header dicts remain. Net deletion of ~490 lines of duplication.
  • client.py: The two export/export_async methods that make direct httpx calls now use self._api_config.get_default_headers().
  • cli/main.py: Includes all three hh-sdk-* headers with the same graceful version fallback pattern.
  • initialization.py: OTLP exporter headers include all three hh-sdk-* headers via the local _get_sdk_version() helper.
  • Tests: 9 unit tests in test_api_config_headers.py — well-structured, covers the key behaviors.
  • CHANGELOG: Customer-facing, concise.

No new issues found. LGTM.


@skylarmb skylarmb marked this pull request as draft February 26, 2026 00:30
Comment thread scripts/templates/apiconfig_pydantic_2.jinja2
@skylarmb skylarmb marked this pull request as ready for review February 26, 2026 00:47
@skylarmb skylarmb merged commit e783f27 into federated-sdk-release-candidate Feb 26, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants