-
Notifications
You must be signed in to change notification settings - Fork 21
refactor(DecoupleLoadFromRuntime): decouple eval load from eval runtime #1244
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
Conversation
| return eval_set, resolved_path | ||
|
|
||
| @staticmethod | ||
| async def load_evaluators( |
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.
This is moved from eval runtime as it needs to be adjacent to load eval set.
| return True | ||
|
|
||
|
|
||
| def _find_agent_model_in_runtime(runtime: UiPathRuntimeProtocol) -> str | None: |
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.
Moved from eval runtime
| f"eval_set_run_id={context.eval_set_run_id}" | ||
| ) | ||
| self.execution_id = ( | ||
| context.job_id or context.eval_set_run_id or str(uuid.uuid4()) |
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.
we need this for a consistent job id before and after suspend resume.
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.
Yes. This logic has been moved to _cli_eval.py right? Am I missing something?
Chibionos
left a comment
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.
execution id being set to job key is important for the suspend resume to recover properly.
| The model name if found, None otherwise. | ||
| """ | ||
| # Check if this runtime implements the protocol | ||
| if isinstance(runtime, LLMAgentRuntimeProtocol): |
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.
We don't need this fallback anymore, the low-code runtime has the model in get_schema() (we can remove the LLMAgentRuntimeProtocol as well)
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.
Can I remove it in a separate small PR? I want this one to just focus on the decoupling,,
Chibionos
left a comment
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.
approved, please ensure the suspend resume sample works properly with your change by running the tools suspend resume sample in langchain repo.
This allows customers to run evals dynamically without having to materialize to files.
d83b481 to
2621a0e
Compare


This allows customers to run evals dynamically without having to materialize to files.
Development Package
uipath pack --nolockto get the latest dev build from this PR (requires version range).