-
Notifications
You must be signed in to change notification settings - Fork 238
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
docs: improve JWT token retrieval documentation #590
Changes from all commits
13069e2
541f9c4
d2bb6f9
dfe8708
8f6ca8a
d0a4ca5
3d1a5a9
1abd561
0f7ad3f
d394a9d
8853464
e2ad872
3e9239d
240e225
4fdcae8
76ba2ea
b897a92
ee9efd3
1a75a54
4e3521b
5eb272e
b4e5b6e
3ea9af2
931570a
102344e
67df03d
45eb7a4
baca1e6
2bf2c75
fdcac1a
889a234
61927aa
ff6a56e
344b4dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -57,6 +57,14 @@ def get_status(code: int) -> HttpStatus: | |||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
class HttpClient: | ||||||||||||||||||||||||||||
_session: Optional[requests.Session] = None | ||||||||||||||||||||||||||||
_base_url: str = "https://api.agentops.ai" | ||||||||||||||||||||||||||||
_test_mode: bool = False | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
@classmethod | ||||||||||||||||||||||||||||
def set_base_url(cls, url: str) -> None: | ||||||||||||||||||||||||||||
"""Set the base URL for all HTTP requests""" | ||||||||||||||||||||||||||||
cls._base_url = url | ||||||||||||||||||||||||||||
cls._test_mode = not url # Enable test mode when base URL is empty | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
@classmethod | ||||||||||||||||||||||||||||
def get_session(cls) -> requests.Session: | ||||||||||||||||||||||||||||
|
@@ -95,38 +103,57 @@ def _prepare_headers( | |||||||||||||||||||||||||||
custom_headers: Optional[dict] = None, | ||||||||||||||||||||||||||||
) -> dict: | ||||||||||||||||||||||||||||
"""Prepare headers for the request""" | ||||||||||||||||||||||||||||
headers = JSON_HEADER.copy() | ||||||||||||||||||||||||||||
headers = {k.lower(): v for k, v in JSON_HEADER.items()} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if api_key is not None: | ||||||||||||||||||||||||||||
headers["X-Agentops-Api-Key"] = api_key | ||||||||||||||||||||||||||||
headers["x-agentops-api-key"] = api_key | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if parent_key is not None: | ||||||||||||||||||||||||||||
headers["X-Agentops-Parent-Key"] = parent_key | ||||||||||||||||||||||||||||
headers["x-agentops-parent-key"] = parent_key | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if jwt is not None: | ||||||||||||||||||||||||||||
headers["Authorization"] = f"Bearer {jwt}" | ||||||||||||||||||||||||||||
headers["authorization"] = f"Bearer {jwt}" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if custom_headers is not None: | ||||||||||||||||||||||||||||
headers.update(custom_headers) | ||||||||||||||||||||||||||||
headers.update({k.lower(): v for k, v in custom_headers.items()}) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
Comment on lines
113
to
119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Actionable Steps:
By addressing this, you can prevent potential authentication issues and ensure smooth communication with the server. 🛠️ 🔧 Suggested Code Diff: - headers["authorization"] = f"Bearer {jwt}"
+ headers["Authorization"] = f"Bearer {jwt}" 📝 Committable Code Suggestion
Suggested change
|
||||||||||||||||||||||||||||
return headers | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
@classmethod | ||||||||||||||||||||||||||||
def post( | ||||||||||||||||||||||||||||
cls, | ||||||||||||||||||||||||||||
url: str, | ||||||||||||||||||||||||||||
payload: bytes, | ||||||||||||||||||||||||||||
payload: Optional[bytes] = None, | ||||||||||||||||||||||||||||
json_data: Optional[Dict[str, Any]] = None, | ||||||||||||||||||||||||||||
api_key: Optional[str] = None, | ||||||||||||||||||||||||||||
parent_key: Optional[str] = None, | ||||||||||||||||||||||||||||
jwt: Optional[str] = None, | ||||||||||||||||||||||||||||
header: Optional[Dict[str, str]] = None, | ||||||||||||||||||||||||||||
headers: Optional[Dict[str, str]] = None, # Renamed from header to headers | ||||||||||||||||||||||||||||
) -> Response: | ||||||||||||||||||||||||||||
"""Make HTTP POST request using connection pooling""" | ||||||||||||||||||||||||||||
result = Response() | ||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||
headers = cls._prepare_headers(api_key, parent_key, jwt, header) | ||||||||||||||||||||||||||||
headers = cls._prepare_headers(api_key, parent_key, jwt, headers) # Updated parameter name | ||||||||||||||||||||||||||||
session = cls.get_session() | ||||||||||||||||||||||||||||
res = session.post(url, data=payload, headers=headers, timeout=20) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Handle either json_data or payload parameter | ||||||||||||||||||||||||||||
if json_data is not None: | ||||||||||||||||||||||||||||
import json as json_module # Import locally to avoid naming conflict | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
data = json_module.dumps(json_data).encode("utf-8") | ||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||
data = payload if payload is not None else b"{}" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Handle URL based on test mode | ||||||||||||||||||||||||||||
if cls._test_mode: | ||||||||||||||||||||||||||||
# Ensure URL starts with /v2 for test mode | ||||||||||||||||||||||||||||
if not url.startswith("/v2"): | ||||||||||||||||||||||||||||
url = f"/v2{url}" if url.startswith("/") else f"/v2/{url}" | ||||||||||||||||||||||||||||
full_url = f"http://localhost{url}" | ||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||
full_url = f"{cls._base_url}{url}" if url.startswith("/") else url | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
res = session.post(full_url, data=data, headers=headers, timeout=20) | ||||||||||||||||||||||||||||
result.parse(res) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
except requests.exceptions.Timeout: | ||||||||||||||||||||||||||||
|
@@ -166,14 +193,22 @@ def get( | |||||||||||||||||||||||||||
url: str, | ||||||||||||||||||||||||||||
api_key: Optional[str] = None, | ||||||||||||||||||||||||||||
jwt: Optional[str] = None, | ||||||||||||||||||||||||||||
header: Optional[Dict[str, str]] = None, | ||||||||||||||||||||||||||||
headers: Optional[Dict[str, str]] = None, # Renamed from header to headers | ||||||||||||||||||||||||||||
) -> Response: | ||||||||||||||||||||||||||||
"""Make HTTP GET request using connection pooling""" | ||||||||||||||||||||||||||||
result = Response() | ||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||
headers = cls._prepare_headers(api_key, None, jwt, header) | ||||||||||||||||||||||||||||
headers = cls._prepare_headers(api_key, None, jwt, headers) # Updated parameter name | ||||||||||||||||||||||||||||
session = cls.get_session() | ||||||||||||||||||||||||||||
res = session.get(url, headers=headers, timeout=20) | ||||||||||||||||||||||||||||
# Handle URL based on test mode | ||||||||||||||||||||||||||||
if cls._test_mode: | ||||||||||||||||||||||||||||
# Ensure URL starts with /v2 for test mode | ||||||||||||||||||||||||||||
if not url.startswith("/v2"): | ||||||||||||||||||||||||||||
url = f"/v2{url}" if url.startswith("/") else f"/v2/{url}" | ||||||||||||||||||||||||||||
full_url = f"http://localhost{url}" | ||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||
full_url = f"{cls._base_url}{url}" if url.startswith("/") else url | ||||||||||||||||||||||||||||
res = session.get(full_url, headers=headers, timeout=20) | ||||||||||||||||||||||||||||
result.parse(res) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
except requests.exceptions.Timeout: | ||||||||||||||||||||||||||||
|
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.
🤖 Bug Fix:
Handle Session Initialization Failure Gracefully
The current code change introduces a potential logical error by returning
None
if the session fails to start. This can lead to unexpected behavior or crashes if the calling code does not handle theNone
return value properly.Recommendations:
None
, consider raising a specific exception that can be caught by the calling code. This approach provides a clear indication of the failure and allows for more robust error handling.None
, ensure that the calling code checks for this condition and handles it appropriately, possibly by logging an error or retrying the session start.Suggested Code Change:
This change will ensure that the failure to start a session is clearly communicated and can be handled appropriately by the calling code.
🔧 Suggested Code Diff:
📝 Committable Code Suggestion
📜 Guidelines
• Use exceptions for error handling, but avoid assert statements for critical logic