-
Notifications
You must be signed in to change notification settings - Fork 0
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
Copy of PR #435 - make start_session non blocking #23
Changes from all commits
a745cd5
a8aeffb
7a81291
56fda75
5f94950
06a9b68
1c8eab6
10c0f1e
960c44c
6c66d0b
c28f16a
2fcb643
277b483
da4ee87
9a9f8ef
1aee480
6b4e50c
7f98d23
379df1d
180edc7
1fefac3
4c591ff
d0524a6
0a673cd
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 | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -87,16 +87,13 @@ def initialize(self) -> Union[Session, None]: | |||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
self.unsuppress_logs() | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
if self._config.api_key is None: | ||||||||||||||||||||||||||||||||||||||||||||
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. Comment: Potential exposure of sensitive API keys. Solution: Implement validation for API keys to ensure they are in the expected format and not empty before use.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
return logger.error( | ||||||||||||||||||||||||||||||||||||||||||||
"Could not initialize AgentOps client - API Key is missing." | ||||||||||||||||||||||||||||||||||||||||||||
+ "\n\t Find your API key at https://app.agentops.ai/settings/projects" | ||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
self._handle_unclean_exits() | ||||||||||||||||||||||||||||||||||||||||||||
self._initialize_partner_framework() | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
self._initialized = True | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
if self._config.instrument_llm_calls: | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
94
to
99
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. ℹ️ Logic ErrorPotential Missing Initialization LogicThe removal of Commitable Code Suggestion:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -116,7 +113,7 @@ def initialize(self) -> Union[Session, None]: | |||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return session | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
def _initialize_partner_framework(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||
def _initialize_autogen_logger(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||
import autogen | ||||||||||||||||||||||||||||||||||||||||||||
from .partners.autogen_logger import AutogenLogger | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -235,11 +232,30 @@ def start_session( | |||||||||||||||||||||||||||||||||||||||||||
if tags is not None: | ||||||||||||||||||||||||||||||||||||||||||||
session_tags.update(tags) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
def _start_session_callback(session: Session): | ||||||||||||||||||||||||||||||||||||||||||||
if session.is_running: | ||||||||||||||||||||||||||||||||||||||||||||
if len(self._pre_init_queue["agents"]) > 0: | ||||||||||||||||||||||||||||||||||||||||||||
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"] = [] | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
logger.info( | ||||||||||||||||||||||||||||||||||||||||||||
colored( | ||||||||||||||||||||||||||||||||||||||||||||
f"\x1b[34mSession Replay: https://app.agentops.ai/drilldown?session_id={session.session_id}\x1b[0m", | ||||||||||||||||||||||||||||||||||||||||||||
"blue", | ||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||
self._sessions.remove(session) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
session = Session( | ||||||||||||||||||||||||||||||||||||||||||||
session_id=session_id, | ||||||||||||||||||||||||||||||||||||||||||||
tags=list(session_tags), | ||||||||||||||||||||||||||||||||||||||||||||
host_env=get_host_env(self._config.env_data_opt_out), | ||||||||||||||||||||||||||||||||||||||||||||
config=self._config, | ||||||||||||||||||||||||||||||||||||||||||||
callback=_start_session_callback, | ||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
if not session.is_running: | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
232
to
261
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. ℹ️ Performance ImprovementOptimize Callback Usage for Session InitializationThe introduction of Commitable Code Suggestion:@@ -235,11 +232,30 @@ def start_session( if tags is not None: session_tags.update(tags) + def _start_session_callback(session: Session): + if session.is_running: + if len(self._pre_init_queue["agents"]) > 0: + 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"] = [] + + logger.info( + colored( + f"\x1b[34mSession Replay: https://app.agentops.ai/drilldown?session_id={session.session_id}\x1b[0m", + "blue", + ) + ) + else: + self._sessions.remove(session) + session = Session( session_id=session_id, tags=list(session_tags), host_env=get_host_env(self._config.env_data_opt_out), config=self._config, + callback=_start_session_callback, ) if not session.is_running: |
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -305,7 +321,8 @@ def create_agent( | |||||||||||||||||||||||||||||||||||||||||||
self._pre_init_queue["agents"].append( | ||||||||||||||||||||||||||||||||||||||||||||
{"name": name, "agent_id": agent_id} | ||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||
session.create_agent(name=name, agent_id=agent_id) | ||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||
session.create_agent(name=name, agent_id=agent_id) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return agent_id | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
321
to
328
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. ℹ️ Logic ErrorConditional Agent Creation LogicThe addition of an Commitable Code Suggestion:@@ -305,7 +321,8 @@ def create_agent(
|
||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -47,13 +47,11 @@ def get_sys_packages(): | |||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
def get_installed_packages(): | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||
# TODO: test | ||||||||||||||||||||||||||||||||
# TODO: add to opt out | ||||||||||||||||||||||||||||||||
"Installed Packages": { | ||||||||||||||||||||||||||||||||
dist.metadata["Name"]: dist.version | ||||||||||||||||||||||||||||||||
dist.metadata.get("Name"): dist.metadata.get("Version") | ||||||||||||||||||||||||||||||||
for dist in importlib.metadata.distributions() | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
Comment on lines
47
to
57
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. 💻 Code ImprovementUse of
|
def get_installed_packages(): | |
try: | |
return { | |
# TODO: test | |
# TODO: add to opt out | |
"Installed Packages": { | |
dist.metadata["Name"]: dist.version | |
dist.metadata.get("Name"): dist.metadata.get("Version") | |
for dist in importlib.metadata.distributions() | |
} | |
} | |
dist.metadata.get("Name", "Unknown"): dist.metadata.get("Version", "Unknown") |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -81,11 +81,9 @@ def post( | |||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if jwt is not None: | ||||||||||||||||||||||||||||||
JSON_HEADER["Authorization"] = f"Bearer {jwt}" | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
res = request_session.post( | ||||||||||||||||||||||||||||||
url, data=payload, headers=JSON_HEADER, timeout=20 | ||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
result.parse(res) | ||||||||||||||||||||||||||||||
except requests.exceptions.Timeout: | ||||||||||||||||||||||||||||||
result.code = 408 | ||||||||||||||||||||||||||||||
Comment on lines
81
to
89
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. ℹ️ Error HandlingAdd Logging for Timeout ExceptionsThe current implementation catches a except requests.exceptions.Timeout:
+ logging.error(f"Request to {url} timed out.")
result.code = 408 Commitable Code Suggestion:
Suggested change
|
||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,7 +5,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||
import time | ||||||||||||||||||||||||||||||||||||||||||||||||||
from decimal import ROUND_HALF_UP, Decimal | ||||||||||||||||||||||||||||||||||||||||||||||||||
from termcolor import colored | ||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import Optional, List, Union | ||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import Optional, List, Union, Callable | ||||||||||||||||||||||||||||||||||||||||||||||||||
from uuid import UUID, uuid4 | ||||||||||||||||||||||||||||||||||||||||||||||||||
from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -40,6 +40,7 @@ def __init__( | |||||||||||||||||||||||||||||||||||||||||||||||||
config: Configuration, | ||||||||||||||||||||||||||||||||||||||||||||||||||
tags: Optional[List[str]] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||
host_env: Optional[dict] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||
callback: Optional[Callable[["Session"], None]] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||
): | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.end_timestamp = None | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.end_state: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -60,17 +61,13 @@ def __init__( | |||||||||||||||||||||||||||||||||||||||||||||||||
"errors": 0, | ||||||||||||||||||||||||||||||||||||||||||||||||||
"apis": 0, | ||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
self.is_running = False | ||||||||||||||||||||||||||||||||||||||||||||||||||
active_sessions.append(self) | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.stop_flag = threading.Event() | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.thread = threading.Thread(target=self._run) | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.thread = threading.Thread(target=self._run, args=(callback,)) | ||||||||||||||||||||||||||||||||||||||||||||||||||
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. Comment: Unnecessary thread creation in session management. Solution: Consider using a thread pool or asynchronous programming to manage sessions more efficiently.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
self.thread.daemon = True | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.thread.start() | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
self.is_running = self._start_session() | ||||||||||||||||||||||||||||||||||||||||||||||||||
if self.is_running == False: | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.stop_flag.set() | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.thread.join(timeout=1) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
def set_video(self, video: str) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||
Sets a url to the video recording of the session. | ||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
61
to
73
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. ℹ️ Logic ErrorEnsure Session Stops if Initialization FailsThe removal of the initial session start check and immediate stop flag setting could lead to the session thread running even if the session fails to start. This could result in unnecessary resource usage and potential errors. + self.is_running = self._start_session()
+ if self.is_running == False:
+ self.stop_flag.set()
+ self.thread.join(timeout=1) Commitable Code Suggestion:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -274,16 +271,21 @@ def _start_session(self): | |||||||||||||||||||||||||||||||||||||||||||||||||
self.config.parent_key, | ||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||
except ApiServerException as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||
return logger.error(f"Could not start session - {e}") | ||||||||||||||||||||||||||||||||||||||||||||||||||
logger.error(f"Could not start session - {e}") | ||||||||||||||||||||||||||||||||||||||||||||||||||
return False | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
logger.debug(res.body) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
if res.code != 200: | ||||||||||||||||||||||||||||||||||||||||||||||||||
logger.error(f"Could not start session - server error") | ||||||||||||||||||||||||||||||||||||||||||||||||||
return False | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
jwt = res.body.get("jwt", None) | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.jwt = jwt | ||||||||||||||||||||||||||||||||||||||||||||||||||
if jwt is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
logger.error( | ||||||||||||||||||||||||||||||||||||||||||||||||||
f"Could not start session - server could not authenticate your API Key" | ||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||
return False | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
session_url = res.body.get( | ||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
271
to
291
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. 🔒 Security SuggestionSanitize Error Messages in LogsThe error handling for session start has been improved, but ensure that sensitive information is not logged. Consider sanitizing error messages to prevent potential exposure of sensitive data. Commitable Code Suggestion:@@ -274,16 +271,21 @@ def _start_session(self):
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -359,10 +361,19 @@ def _flush_queue(self) -> None: | |||||||||||||||||||||||||||||||||||||||||||||||||
elif event_type == "apis": | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.event_counts["apis"] += 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
def _run(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
def _run(self, callback: Optional[Callable[["Session"], None]] = None) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.is_running = self._start_session() | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
if callback: | ||||||||||||||||||||||||||||||||||||||||||||||||||
callback(self) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
if self.is_running == False: | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.stop_flag.set() | ||||||||||||||||||||||||||||||||||||||||||||||||||
self.thread.join(timeout=1) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
while not self.stop_flag.is_set(): | ||||||||||||||||||||||||||||||||||||||||||||||||||
time.sleep(self.config.max_wait_time / 1000) | ||||||||||||||||||||||||||||||||||||||||||||||||||
if self.queue: | ||||||||||||||||||||||||||||||||||||||||||||||||||
if self.queue and self.jwt is not None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
self._flush_queue() | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
def create_agent(self, name, agent_id): | ||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
361
to
379
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. |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
Entelligence AI Bot v4
ℹ️ Performance Improvement
Initialize Autogen Logger for Enhanced Logging
The addition of
_initialize_autogen_logger()
ensures that the logging mechanism is set up correctly when theautogen
module is present. This change enhances the logging capabilities, allowing for better tracking and debugging of operations related to theautogen
module.Commitable Code Suggestion: