docs: improve JWT token retrieval documentation #590
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Added two new endpoints: - GET /v2/sessions/<session_id>/stats for retrieving session statistics - GET /v2/sessions/<session_id>/export for exporting complete session data Updated SDK documentation with new endpoints and usage examples. Co-Authored-By: Alex Reibman <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
Add "(aside)" to your comment to have me ignore it. |
WalkthroughThis update introduces two new API endpoints for session data management:
Additionally, the SDK documentation has been updated to include these new endpoints with usage examples, ensuring developers can easily integrate these features into their applications. Changes
🔗 Related PRs
InstructionsEmoji Descriptions:
Interact with the Bot:
Execute a command using the format:
Available Commands:
Tips for Using @bot Effectively:
Need More Help?📚 Visit our documentation for detailed guides on using Entelligence.AI. |
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
…ession Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
…Client.post Co-Authored-By: Alex Reibman <[email protected]>
…alization in tests Co-Authored-By: Alex Reibman <[email protected]>
…t files Co-Authored-By: Alex Reibman <[email protected]>
…ruff formatting Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 14909941 | Triggered | Generic High Entropy Secret | 1abd561 | tests/test_session.py | View secret |
| 14909942 | Triggered | Bearer Token | 3d1a5a9 | tests/test_session.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| session = None | ||
| if self._config.auto_start_session: | ||
| session = self.start_session() | ||
| if not session or not session.is_running: | ||
| logger.error("Failed to automatically start session") | ||
| return None | ||
|
|
||
| if session: | ||
| if session and self._pre_init_queue["agents"]: | ||
| for agent_args in self._pre_init_queue["agents"]: | ||
| session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"]) | ||
| self._pre_init_queue["agents"] = [] |
There was a problem hiding this comment.
🤖 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 the None return value properly.
Recommendations:
- Exception Handling: Instead of returning
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. - Error Messaging: If you choose to return
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:
- Implement exception handling to provide a more informative error response.
session = None
if self._config.auto_start_session:
session = self.start_session()
if not session or not session.is_running:
logger.error("Failed to automatically start session")
- return None
+ raise RuntimeError("Session could not be started")
if session and self._pre_init_queue["agents"]:
for agent_args in self._pre_init_queue["agents"]:
session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
self._pre_init_queue["agents"] = []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:
session = None
if self._config.auto_start_session:
session = self.start_session()
if not session or not session.is_running:
logger.error("Failed to automatically start session")
- return None
+ raise RuntimeError("Session could not be started")
if session and self._pre_init_queue["agents"]:
for agent_args in self._pre_init_queue["agents"]:
session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"])
self._pre_init_queue["agents"] = []📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| session = None | |
| if self._config.auto_start_session: | |
| session = self.start_session() | |
| if not session or not session.is_running: | |
| logger.error("Failed to automatically start session") | |
| return None | |
| if session: | |
| if session and self._pre_init_queue["agents"]: | |
| for agent_args in self._pre_init_queue["agents"]: | |
| session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"]) | |
| self._pre_init_queue["agents"] = [] | |
| session = None | |
| if self._config.auto_start_session: | |
| session = self.start_session() | |
| if not session or not session.is_running: | |
| logger.error("Failed to automatically start session") | |
| raise RuntimeError("Session could not be started") | |
| if session and self._pre_init_queue["agents"]: | |
| for agent_args in self._pre_init_queue["agents"]: | |
| session.create_agent(name=agent_args["name"], agent_id=agent_args["agent_id"]) | |
| self._pre_init_queue["agents"] = [] |
📜 Guidelines
• Use exceptions for error handling, but avoid assert statements for critical logic
|
|
||
| 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()}) | ||
|
|
There was a problem hiding this comment.
Ensure Case-Insensitive Header Handling
The change from 'Authorization' to 'authorization' in the header key may lead to issues if the server does not handle headers in a case-insensitive manner, despite the HTTP/1.1 specification stating that headers should be case-insensitive. This could result in authentication failures if the server expects a specific case.
Actionable Steps:
- Verify Server Behavior: Confirm that the server processes headers in a case-insensitive manner. This is crucial to ensure compatibility.
- Revert Header Key Case: If the server is case-sensitive, revert the header key to 'Authorization'.
- Server Compatibility: Ensure that any server-side code or third-party services interacting with this client are compatible with the header case used.
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
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| 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()}) | |
| if jwt is not None: | |
| headers["Authorization"] = f"Bearer {jwt}" | |
| if custom_headers is not None: | |
| headers.update({k.lower(): v for k, v in custom_headers.items()}) | |
| return headers |
tests/test_pre_init.py
Outdated
| def mock_req(): | ||
| """Set up mock requests.""" | ||
| with requests_mock.Mocker() as m: | ||
| url = "https://api.agentops.ai" | ||
| m.post(url + "/v2/create_agent", json={"status": "success"}) | ||
| m.post(url + "/v2/update_session", json={"status": "success", "token_cost": 5}) | ||
| m.post(url + "/v2/create_session", json={"status": "success", "jwt": "some_jwt"}) | ||
| base_url = "https://api.agentops.ai/v2" | ||
| api_key = "2a458d3f-5bd7-4798-b862-7d9a54515689" | ||
|
|
||
| def match_headers(request): | ||
| headers = {k.lower(): v for k, v in request.headers.items()} | ||
| return headers.get("x-agentops-api-key") == api_key and ( | ||
| headers.get("authorization", "").startswith("Bearer ") or request.path == "/v2/sessions/start" | ||
| ) |
There was a problem hiding this comment.
🤖 Bug Fix:
Remove Hardcoded API Key for Security
The current code introduces a hardcoded API key, which poses a significant security risk. Hardcoding sensitive information like API keys can lead to unauthorized access if the code is exposed. To mitigate this risk, it's crucial to remove the hardcoded API key and instead use environment variables or a secure vault to store and access sensitive information. This approach ensures that the API key is not exposed in the codebase and can be managed securely.
Actionable Steps:
- Remove the hardcoded API key from the code.
- Use environment variables to store the API key securely.
- Update the code to retrieve the API key from the environment variables at runtime.
- Ensure that the environment variables are set up correctly in the deployment environment.
🔧 Suggested Code Diff:
- api_key = "2a458d3f-5bd7-4798-b862-7d9a54515689"
+ import os
+ api_key = os.getenv('AGENTOPS_API_KEY')
+
+ if not api_key:
+ raise ValueError("API key not found. Please set the AGENTOPS_API_KEY environment variable.")📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| def mock_req(): | |
| """Set up mock requests.""" | |
| with requests_mock.Mocker() as m: | |
| url = "https://api.agentops.ai" | |
| m.post(url + "/v2/create_agent", json={"status": "success"}) | |
| m.post(url + "/v2/update_session", json={"status": "success", "token_cost": 5}) | |
| m.post(url + "/v2/create_session", json={"status": "success", "jwt": "some_jwt"}) | |
| base_url = "https://api.agentops.ai/v2" | |
| api_key = "2a458d3f-5bd7-4798-b862-7d9a54515689" | |
| def match_headers(request): | |
| headers = {k.lower(): v for k, v in request.headers.items()} | |
| return headers.get("x-agentops-api-key") == api_key and ( | |
| headers.get("authorization", "").startswith("Bearer ") or request.path == "/v2/sessions/start" | |
| ) | |
| import os | |
| import requests_mock | |
| def setup_mock_requests(): | |
| """Set up mock requests.""" | |
| with requests_mock.Mocker() as m: | |
| base_url = "https://api.agentops.ai/v2" | |
| api_key = os.getenv('AGENTOPS_API_KEY') | |
| if not api_key: | |
| raise ValueError("API key not found. Please set the AGENTOPS_API_KEY environment variable.") | |
| def match_headers(request): | |
| headers = {k.lower(): v for k, v in request.headers.items()} | |
| return headers.get("x-agentops-api-key") == api_key and ( | |
| headers.get("authorization", "").startswith("Bearer ") or request.path == "/v2/sessions/start" | |
| ) | |
| m.post(base_url + "/create_agent", json={"status": "success"}, additional_matcher=match_headers) | |
| m.post(base_url + "/update_session", json={"status": "success", "token_cost": 5}, additional_matcher=match_headers) | |
| m.post(base_url + "/create_session", json={"status": "success", "jwt": "some_jwt"}, additional_matcher=match_headers) |
📜 Guidelines
• Use docstrings for modules, functions, and classes
• Follow PEP 8 style guide for consistent code formatting
| agentops.end_all_sessions() | ||
|
|
||
| def test_add_tags_with_string(self, mock_req): | ||
| agentops.start_session() | ||
| agentops.add_tags("wrong-type-tags") | ||
| session = agentops.start_session() | ||
| session.add_tags("wrong-type-tags") | ||
| time.sleep(0.1) # Wait for request to complete | ||
|
|
||
| request_json = mock_req.last_request.json() | ||
| assert request_json["session"]["tags"] == ["wrong-type-tags"] | ||
| assert "wrong-type-tags" in request_json["session"]["tags"] | ||
|
|
||
| def test_session_add_tags_with_string(self, mock_req): | ||
| session = agentops.start_session() |
There was a problem hiding this comment.
Ensure Consistent Behavior in Refactored Code
The refactoring from agentops.add_tags to session.add_tags introduces a potential logical error. It's crucial to verify that the session object is correctly initialized and that the add_tags method behaves as expected. This change could affect the session's state or the way tags are added if the session object does not mirror the previous implementation's behavior.
Actionable Steps:
- Verify Initialization: Ensure that the
sessionobject is properly initialized before callingadd_tags. - Behavior Consistency: Confirm that
session.add_tagsperforms the same operations asagentops.add_tags. This might involve reviewing the method's implementation or adding additional tests. - Testing: Add unit tests to cover edge cases and ensure that tags are added correctly to the session.
By addressing these points, you can mitigate the risk of introducing bugs due to this refactoring. 🛠️
📜 Guidelines
• Write unit tests for your code
• Use meaningful variable and function names following specific naming conventions
tests/test_session.py
Outdated
|
|
||
| # 5 requests: check_for_updates, 2 start_session, 2 record_event | ||
| assert len(mock_req.request_history) == 5 | ||
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | ||
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | ||
| request_json = mock_req.last_request.json() | ||
| assert request_json["events"][0]["event_type"] == self.event_type | ||
|
|
There was a problem hiding this comment.
Potential Case Sensitivity Issue with HTTP Header
The change in the header key from 'Authorization' to 'authorization' could lead to issues if the server handling the requests is case-sensitive. Although HTTP headers are generally case-insensitive, some server implementations might not strictly adhere to this standard. This could result in authentication failures if the server expects a specific case for the header key.
Actionable Steps:
- Verify Server Behavior: Check if the server handling these requests is case-sensitive regarding header keys.
- Revert if Necessary: If the server is case-sensitive, revert the header key to 'Authorization'.
- Ensure Consistency: If the server is not case-sensitive, ensure that all parts of the system consistently use the new case 'authorization'.
By addressing this issue, you can prevent potential authentication errors and ensure the robustness of the system. 🛡️
🔧 Suggested Code Diff:
- assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}"
+ assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}"📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # 5 requests: check_for_updates, 2 start_session, 2 record_event | |
| assert len(mock_req.request_history) == 5 | |
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | |
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["events"][0]["event_type"] == self.event_type | |
| # 5 requests: check_for_updates, 2 start_session, 2 record_event | |
| assert len(mock_req.request_history) == 5 | |
| assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["events"][0]["event_type"] == self.event_type |
📜 Guidelines
• Follow PEP 8 style guide for consistent code formatting
|
|
||
| # Additional end session request | ||
| assert len(mock_req.request_history) == 6 | ||
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | ||
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | ||
| request_json = mock_req.last_request.json() | ||
| assert request_json["session"]["end_state"] == end_state | ||
| assert len(request_json["session"]["tags"]) == 0 |
There was a problem hiding this comment.
Ensure HTTP Header Case Sensitivity Compliance
The change in the header key from 'Authorization' to 'authorization' could lead to issues if the server is case-sensitive regarding HTTP header names. Although HTTP/1.1 specifies that headers are case-insensitive, not all server implementations adhere to this strictly. This could result in authentication failures if the server expects a specific case.
Actionable Steps:
- Verify Server Behavior: Check if the server is case-sensitive with HTTP headers. If it is, ensure the header key matches the expected case.
- Test Thoroughly: Conduct tests to confirm that the server correctly handles case-insensitive headers.
- Documentation: Update any relevant documentation to reflect the expected header case to prevent future issues.
By addressing this, you ensure robust authentication handling and prevent potential failures. 🛡️
| session_2.end_session(end_state) | ||
| # Additional end session request | ||
| assert len(mock_req.request_history) == 7 | ||
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | ||
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | ||
| request_json = mock_req.last_request.json() | ||
| assert request_json["session"]["end_state"] == end_state | ||
| assert len(request_json["session"]["tags"]) == 0 |
There was a problem hiding this comment.
Potential Case Sensitivity Issue in HTTP Header
The change in the header key from 'Authorization' to 'authorization' could lead to issues if the server is case-sensitive regarding header keys. Although HTTP headers are generally case-insensitive, some server implementations might not strictly adhere to this, potentially causing authentication failures.
Actionable Steps:
- Verify Server Behavior: Check if the server handling the request is case-sensitive to header keys. This can be done by consulting the server documentation or testing with different header cases.
- Ensure Consistency: If the server is case-sensitive, revert the header key to 'Authorization' to ensure compatibility.
- Testing: Conduct thorough testing to confirm that the change does not affect request processing.
By addressing this, you can prevent potential authentication issues and ensure the robustness of the application. 🛡️
🔧 Suggested Code Diff:
- assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}"
+ assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}"📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| session_2.end_session(end_state) | |
| # Additional end session request | |
| assert len(mock_req.request_history) == 7 | |
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | |
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["session"]["end_state"] == end_state | |
| assert len(request_json["session"]["tags"]) == 0 | |
| session_2.end_session(end_state) # Additional end session request | |
| assert len(mock_req.request_history) == 7 | |
| assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["session"]["end_state"] == end_state | |
| assert len(request_json["session"]["tags"]) == 0 |
Co-Authored-By: Alex Reibman <[email protected]>
…test files Co-Authored-By: Alex Reibman <[email protected]>
Co-Authored-By: Alex Reibman <[email protected]>
…ialization Co-Authored-By: Alex Reibman <[email protected]>
…ion_url with session_url Co-Authored-By: Alex Reibman <[email protected]>
…est initialization Co-Authored-By: Alex Reibman <[email protected]>
…entops-api-key Co-Authored-By: Alex Reibman <[email protected]>
| ) | ||
| return self.token_cost | ||
|
|
||
| def add_tags(self, tags: List[str]) -> None: | ||
| """ | ||
| Append to session tags at runtime. | ||
| """ | ||
| if not self.is_running: | ||
| return | ||
|
|
||
| if not (isinstance(tags, list) and all(isinstance(item, str) for item in tags)): | ||
| if isinstance(tags, str): | ||
| tags = [tags] | ||
|
|
||
| # Initialize tags if None | ||
| if self.tags is None: | ||
| self.tags = [] | ||
|
|
||
| # Add new tags that don't exist | ||
| for tag in tags: | ||
| if tag not in self.tags: | ||
| self.tags.append(tag) | ||
| def add_tags(self, tags: Union[str, List[str]]) -> None: | ||
| """Add tags to the session. | ||
|
|
||
| # Update session state immediately | ||
| self._update_session() | ||
|
|
||
| def set_tags(self, tags): | ||
| """Set session tags, replacing any existing tags""" | ||
| if not self.is_running: | ||
| return | ||
| Args: | ||
| tags (Union[str, List[str]]): A string or list of strings to add as tags. | ||
| """ | ||
| if isinstance(tags, str): | ||
| new_tags = [tags] | ||
| elif isinstance(tags, (list, tuple)): | ||
| new_tags = [str(tag) for tag in tags] # Ensure all tags are strings | ||
| else: | ||
| raise ValueError("Tags must be a string or list of strings") |
There was a problem hiding this comment.
🤖 Bug Fix:
Handling None in add_tags Method
The refactored add_tags method introduces a potential bug by not handling the case where tags is None. This oversight can lead to an AttributeError if add_tags is called with None when self.tags is also None. To maintain backward compatibility and prevent errors, it's crucial to add a check for tags being None and initialize self.tags to an empty list if it is None. This ensures the method behaves as expected and does not break existing functionality.
🔧 Suggested Code Diff:
def add_tags(self, tags: Union[str, List[str]]) -> None:
"""Add tags to the session.
Args:
tags (Union[str, List[str]]): A string or list of strings to add as tags.
"""
+ if tags is None:
+ return
if isinstance(tags, str):
new_tags = [tags]
elif isinstance(tags, (list, tuple)):
new_tags = [str(tag) for tag in tags] # Ensure all tags are strings
else:
raise ValueError("Tags must be a string or list of strings")
self.tags.extend(new_tags)
self.tags = list(set(self.tags)) # Remove duplicates
if self.is_running:
self._update_session()📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ) | |
| return self.token_cost | |
| def add_tags(self, tags: List[str]) -> None: | |
| """ | |
| Append to session tags at runtime. | |
| """ | |
| if not self.is_running: | |
| return | |
| if not (isinstance(tags, list) and all(isinstance(item, str) for item in tags)): | |
| if isinstance(tags, str): | |
| tags = [tags] | |
| # Initialize tags if None | |
| if self.tags is None: | |
| self.tags = [] | |
| # Add new tags that don't exist | |
| for tag in tags: | |
| if tag not in self.tags: | |
| self.tags.append(tag) | |
| def add_tags(self, tags: Union[str, List[str]]) -> None: | |
| """Add tags to the session. | |
| # Update session state immediately | |
| self._update_session() | |
| def set_tags(self, tags): | |
| """Set session tags, replacing any existing tags""" | |
| if not self.is_running: | |
| return | |
| Args: | |
| tags (Union[str, List[str]]): A string or list of strings to add as tags. | |
| """ | |
| if isinstance(tags, str): | |
| new_tags = [tags] | |
| elif isinstance(tags, (list, tuple)): | |
| new_tags = [str(tag) for tag in tags] # Ensure all tags are strings | |
| else: | |
| raise ValueError("Tags must be a string or list of strings") | |
| def add_tags(self, tags: Union[str, List[str]]) -> None: | |
| """Add tags to the session. | |
| Args: | |
| tags (Union[str, List[str]]): A string or list of strings to add as tags. | |
| """ | |
| if tags is None: | |
| return | |
| if isinstance(tags, str): | |
| new_tags = [tags] | |
| elif isinstance(tags, (list, tuple)): | |
| new_tags = [str(tag) for tag in tags] # Ensure all tags are strings | |
| else: | |
| raise ValueError("Tags must be a string or list of strings") | |
| if self.tags is None: | |
| self.tags = [] | |
| self.tags.extend(new_tags) | |
| self.tags = list(set(self.tags)) # Remove duplicates | |
| if self.is_running: | |
| self._update_session() |
| def _reauthorize_jwt(self) -> Union[str, None]: | ||
| with self._lock: | ||
| payload = {"session_id": self.session_id} | ||
| serialized_payload = json.dumps(filter_unjsonable(payload)).encode("utf-8") | ||
| res = HttpClient.post( | ||
| f"{self.config.endpoint}/v2/reauthorize_jwt", | ||
| serialized_payload, | ||
| self.config.api_key, | ||
| json_data=payload, | ||
| api_key=self.config.api_key, | ||
| ) | ||
|
|
||
| logger.debug(res.body) |
There was a problem hiding this comment.
Ensure HttpClient.post Method Compatibility
The refactoring of the HttpClient.post method call to use json_data and api_key parameters instead of serialized_payload and self.config.api_key introduces a potential logical error. This change assumes that the HttpClient.post method is designed to handle these parameters correctly. If not, it could lead to incorrect API calls and failures in reauthorizing JWTs.
Actionable Steps:
- Verify Method Compatibility: Ensure that the
HttpClient.postmethod is updated to acceptjson_dataandapi_keyparameters. This includes verifying that the method correctly serializes the payload and uses the API key as expected by the server. - Test Thoroughly: Conduct thorough testing to confirm that the API calls are functioning as intended and that JWT reauthorization is successful.
- Documentation: Update any relevant documentation to reflect changes in how the
HttpClient.postmethod should be used.
By addressing these points, you can mitigate the risk of logical errors and ensure the robustness of the JWT reauthorization process. 🛠️
| "name": name, | ||
| } | ||
|
|
||
| serialized_payload = safe_serialize(payload).encode("utf-8") | ||
| try: | ||
| HttpClient.post( | ||
| f"{self.config.endpoint}/v2/create_agent", | ||
| serialized_payload, | ||
| json_data=payload, | ||
| api_key=self.config.api_key, | ||
| jwt=self.jwt, | ||
| ) |
There was a problem hiding this comment.
🤖 Bug Fix:
Ensure HttpClient.post Method Compatibility with JSON Data
The change from using serialized_payload to json_data in the HttpClient.post method call could introduce a logical error if the method is not designed to handle JSON objects directly. This could lead to failures in creating an agent, which is a critical functionality.
Actionable Steps:
- Verify Method Compatibility: Check the implementation of the
HttpClient.postmethod to confirm whether it can accept JSON objects directly. - Revert or Update: If the method cannot handle JSON objects, consider reverting to using
serialized_payload. Alternatively, update theHttpClient.postmethod to support JSON data if feasible.
Ensuring the method's compatibility with JSON data is crucial to maintain the integrity of the HTTP request functionality. 🛠️
|
|
||
| # Additional end session request | ||
| assert len(mock_req.request_history) == 6 | ||
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | ||
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | ||
| request_json = mock_req.last_request.json() | ||
| assert request_json["session"]["end_state"] == end_state | ||
| assert len(request_json["session"]["tags"]) == 0 |
There was a problem hiding this comment.
Ensure Header Case Consistency for Authorization
The change in the header key from 'Authorization' to 'authorization' could potentially cause issues if the server is case-sensitive, even though HTTP headers are generally case-insensitive. It's crucial to verify that the server can handle headers regardless of case. If the server is case-sensitive, you should either revert to the original case or update the server to handle headers in a case-insensitive manner.
Actionable Steps:
- Confirm with the server team or documentation that the server processes headers in a case-insensitive way.
- If the server is case-sensitive, revert the header key to 'Authorization'.
- Consider adding a test case to ensure that headers are correctly processed by the server.
🔧 Suggested Code Diff:
- assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}"
+ assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}"📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # Additional end session request | |
| assert len(mock_req.request_history) == 6 | |
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | |
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["session"]["end_state"] == end_state | |
| assert len(request_json["session"]["tags"]) == 0 | |
| # Additional end session request | |
| assert len(mock_req.request_history) == 6 | |
| assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["session"]["end_state"] == end_state | |
| assert len(request_json["session"]["tags"]) == 0 |
| session_2.end_session(end_state) | ||
| # Additional end session request | ||
| assert len(mock_req.request_history) == 7 | ||
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | ||
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | ||
| request_json = mock_req.last_request.json() | ||
| assert request_json["session"]["end_state"] == end_state | ||
| assert len(request_json["session"]["tags"]) == 0 |
There was a problem hiding this comment.
Case Sensitivity in HTTP Headers
The change in the header key from 'Authorization' to 'authorization' could lead to issues if the server is case-sensitive, even though HTTP headers are generally case-insensitive. It's crucial to verify that the server can handle headers in different cases. If the server is case-sensitive, revert the header key to the expected case. Additionally, consider adding a test to ensure the server's behavior with different header cases.
🔧 Suggested Code Diff:
- assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}"
+ assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}"📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| session_2.end_session(end_state) | |
| # Additional end session request | |
| assert len(mock_req.request_history) == 7 | |
| assert mock_req.last_request.headers["Authorization"] == "Bearer some_jwt" | |
| assert mock_req.last_request.headers["authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["session"]["end_state"] == end_state | |
| assert len(request_json["session"]["tags"]) == 0 | |
| session_2.end_session(end_state) # Additional end session request | |
| assert len(mock_req.request_history) == 7 | |
| assert mock_req.last_request.headers["Authorization"] == f"Bearer {self.api_key}" | |
| request_json = mock_req.last_request.json() | |
| assert request_json["session"]["end_state"] == end_state | |
| assert len(request_json["session"]["tags"]) == 0 |
Co-Authored-By: Alex Reibman <[email protected]>
devin-ai-integration
bot
changed the title
|
Closing in favor of a documentation-only PR |
devin-ai-integration
bot
deleted the
devin/1734428749-add-session-export-apis
branch
This PR improves the documentation around JWT token retrieval in the AgentOps SDK.
Changes
Testing
No code changes, documentation only.
Link to Devin run: https://app.devin.ai/sessions/ad7c503faf0142f69a5cb28ae2abe0cf