add new tool definitions #1171
add new tool definitions #1171
Conversation
github-actions
bot
added
test:uipath-langchain
src/uipath/platform/context_grounding/_context_grounding_service.py
Outdated
Show resolved
Hide resolved
CalebMartinUiPath
force-pushed
the
caleb/jit-ecs-sdk
branch
3 times, most recently
from
88177a3 to
feb7340
Compare
CalebMartinUiPath
force-pushed
the
caleb/add-tool-definitions
branch
2 times, most recently
from
2d41733 to
5bdbf83
Compare
| destination_path: str | ||
| index_folder_key: str | None = None | ||
| index_folder_path: str | None = None | ||
| is_ephemeral: bool | None = None |
There was a problem hiding this comment.
as to not break existing code using this. If not provided we will assume it is not an ephemeral index
| index_name, folder_key=folder_key, folder_path=folder_path | ||
| ) | ||
| if index and index.in_progress_ingestion(): | ||
| raise IngestionInProgressException(index_name=index_name) |
There was a problem hiding this comment.
why are we checking this? handle the 400
There was a problem hiding this comment.
when developing an agent, we need a reliable way of identifying if the 400 comes from index being ingested or there is some other problem.
there is no other way of handling ingestions (if you don t want to suspend-resume the agent).
check this langchain sample https://github.com/UiPath/uipath-langchain-python/blob/main/samples/RAG-quiz-generator/src/agents/quiz-generator-RAG-agent.py#L121
There was a problem hiding this comment.
I don't understand the link you sent - it checks for ingesting for 25s then gives up - it seems very fragile.
creation of batch transform can fail with 400s for many different reasons (invalid args, failed ingestion, ingestion in progress, etc) - we are not going to check every single one before calling the POST, why do it for already ingesting?
If you need a way to know before calling create_x that the index is ready then why not use the MB event, or have some polling function that only continues before calling creation.
| index_name: str, | ||
| prompt: Annotated[str, Field(max_length=250000)], | ||
| output_columns: list[BatchTransformOutputColumn], | ||
| storage_bucket_folder_path_prefix: Annotated[ |
There was a problem hiding this comment.
folder path prefix is not only for storage buckets
There was a problem hiding this comment.
@radu-mocanu is it ok to change this? will that break any existing code?
There was a problem hiding this comment.
I need some more context here. can we have folder path filtering for attachments?
There was a problem hiding this comment.
but yes, this would break the method interface.
There was a problem hiding this comment.
I should have been more clear - it was more of a naming nit - folder path prefix can be applicable for one drives, google drives, storage buckets ...
| output_columns (list[BatchTransformOutputColumn]): The output columns to add into the csv. | ||
| storage_bucket_folder_path_prefix (str): The prefix pattern for filtering files in the storage bucket. Use "*" to include all files. Defaults to "*". | ||
| enable_web_search_grounding (Optional[bool]): Whether to enable web search. Defaults to False. | ||
| index_id (str): The id of the context index to search in, used in place of name if present |
There was a problem hiding this comment.
I feel like we can do better encapsulation than having a comment indicate when or when not to use an id. What about overrides? or 2 seperate functions
There was a problem hiding this comment.
@JeremyReist2 this is the pattern we have in place for other methods as well
however it is not a good pattern to have silent precedence (id over name). let s validate the arguments as we do in the add_to_index method
CalebMartinUiPath
force-pushed
the
caleb/add-tool-definitions
branch
from
838c804 to
83e6681
Compare
CalebMartinUiPath
force-pushed
the
caleb/add-tool-definitions
branch
from
83e6681 to
b2431e8
Compare
JeremyReist2
left a comment
JeremyReist2
left a comment
There was a problem hiding this comment.
approving to unblock @CalebMartinUiPath - I disagree with pattern in use for calling another API to throw ingestion errors in create calls - create should only be called after ingestion and if you need to poll or wait for MB before calling create then do so.
Forcing us to remember in every create_x call to check for ingestion and throw that error is a bad pattern, adds duplication, and will break down the road when someone forgets.
3 participants
add new tool definitions needed for the langchain tools