feat: switch to API key auth for Antenna endpoints#136
Open
Conversation
Replace user token auth (Authorization: Token) with API key auth (Authorization: Api-Key) for all Antenna API requests. Remove processing_service_name from registration (service is now identified by its API key). Add client_info metadata to pipeline registration. - Replace antenna_api_auth_token setting with antenna_api_key - Remove antenna_service_name setting (managed in Antenna now) - Update get_http_session to use Api-Key header - Add client_info (hostname, software, version, platform) to registration - Remove get_full_service_name helper (no longer needed) - Update all callers, tests, and benchmark Companion to RolnickLab/antenna#1194 Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRenamed Antenna authentication from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
trapdata/antenna/result_posting.py (1)
63-63:⚠️ Potential issue | 🟡 MinorMinor docstring inconsistency: example still references
auth_token.The class docstring example should be updated to reflect the new parameter name.
📝 Suggested fix
Example: poster = ResultPoster(max_pending=10) - poster.post_async(base_url, auth_token, job_id, results) + poster.post_async(base_url, api_key, job_id, results) metrics = poster.get_metrics() poster.shutdown()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@trapdata/antenna/result_posting.py` at line 63, The class docstring example still uses the old name auth_token; update the example so its call to poster.post_async matches the actual parameter name in the current poster.post_async signature (replace auth_token with the current name used in the function signature), ensuring the example parameters (base_url, <current_auth_param>, job_id, results) match the implementation in result_posting.py.
🧹 Nitpick comments (1)
trapdata/antenna/schemas.py (1)
96-96: Consider using a TypedDict forclient_infoto improve type safety.The
_build_client_info()function intrapdata/antenna/registration.py:28-36produces a specific shape withhostname,software,version, andplatformkeys. A TypedDict would provide better IDE support and catch typos.💡 Optional improvement
from typing import TypedDict class ClientInfo(TypedDict, total=False): hostname: str software: str version: str platform: str class AsyncPipelineRegistrationRequest(pydantic.BaseModel): # ... client_info: ClientInfo | None = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@trapdata/antenna/schemas.py` at line 96, Replace the untyped client_info: dict | None on AsyncPipelineRegistrationRequest with a TypedDict to mirror the shape built by _build_client_info; define a ClientInfo TypedDict (total=False) with keys hostname, software, version, platform (all str) and change the field type to ClientInfo | None so IDEs and type checkers catch mismatches and typos when constructing client_info in _build_client_info.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@trapdata/antenna/registration.py`:
- Around line 24-25: The except Exception in the _get_version function is too
broad; narrow it to only the expected metadata/lookup failures by catching
specific exceptions (e.g., importlib.metadata.PackageNotFoundError and other
lookup-related errors like KeyError/AttributeError) instead of Exception, and
import PackageNotFoundError from importlib.metadata (or reference it fully) so
the handler returns "unknown" only for those expected failure modes.
---
Outside diff comments:
In `@trapdata/antenna/result_posting.py`:
- Line 63: The class docstring example still uses the old name auth_token;
update the example so its call to poster.post_async matches the actual parameter
name in the current poster.post_async signature (replace auth_token with the
current name used in the function signature), ensuring the example parameters
(base_url, <current_auth_param>, job_id, results) match the implementation in
result_posting.py.
---
Nitpick comments:
In `@trapdata/antenna/schemas.py`:
- Line 96: Replace the untyped client_info: dict | None on
AsyncPipelineRegistrationRequest with a TypedDict to mirror the shape built by
_build_client_info; define a ClientInfo TypedDict (total=False) with keys
hostname, software, version, platform (all str) and change the field type to
ClientInfo | None so IDEs and type checkers catch mismatches and typos when
constructing client_info in _build_client_info.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18a4f370-f380-4a34-965d-c9fc905943b4
📒 Files selected for processing (12)
trapdata/antenna/benchmark.pytrapdata/antenna/client.pytrapdata/antenna/datasets.pytrapdata/antenna/registration.pytrapdata/antenna/result_posting.pytrapdata/antenna/schemas.pytrapdata/antenna/tests/test_memory_leak.pytrapdata/antenna/tests/test_worker.pytrapdata/antenna/worker.pytrapdata/api/utils.pytrapdata/cli/worker.pytrapdata/settings.py
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switch from user token auth (
Authorization: Token) to API key auth (Authorization: Api-Key) for all Antenna API requests. The processing service is now identified by its API key rather than by name.antenna_api_auth_tokensetting withantenna_api_key(env var:AMI_ANTENNA_API_KEY)antenna_service_namesetting (service name is now managed in Antenna admin)processing_service_namefrom pipeline registration request bodyclient_infometadata (hostname, software, version, platform) to pipeline registrationget_full_service_namehelperCompanion to RolnickLab/antenna#1194
Migration
POST /api/v2/processing-services/{id}/generate_key/AMI_ANTENNA_API_AUTH_TOKENwithAMI_ANTENNA_API_KEYin your.envfile or environmentAMI_ANTENNA_SERVICE_NAMEfrom your configuration (no longer used)Test plan
ami worker register --project <id>registers pipelines with API key authami worker --pipeline <slug>fetches tasks and posts results with API key authtest_ml_job_e2emanagement command on Antenna sidepytest trapdata/antenna/tests/)🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
New Features