Skip to content
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

[community] Add timeout control and retry for UC tool execution #26645

Merged
merged 7 commits into from
Oct 9, 2024

Conversation

serena-ruan
Copy link
Contributor

@serena-ruan serena-ruan commented Sep 19, 2024

Add timeout at client side for UCFunctionToolkit and add retry logic.
Users could specify environment variable UC_TOOL_CLIENT_EXECUTION_TIMEOUT to increase the timeout value for retrying to get the execution response if the status is pending. Default timeout value is 120s.

  • Add tests and docs: If you're adding a new integration, please include
    1. a test for the integration, preferably unit tests that do not rely on network access,
    2. an example notebook showing its use. It lives in docs/docs/integrations directory.

Tested in Databricks:
image

Additional guidelines:

  • Make sure optional dependencies are imported within a function.
  • Please do not add dependencies to pyproject.toml files (even optional ones) unless they are required for unit tests.
  • Most PRs should not touch more than one package.
  • Changes should be backwards compatible.
  • If you are adding something to community, do not re-import it in langchain.

If no one reviews your PR within a few days, please @-mention one of baskaryan, efriis, eyurtsev, ccurme, vbarda, hwchase17.

Signed-off-by: serena-ruan <[email protected]>
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 19, 2024
Copy link

vercel bot commented Sep 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2024 6:31am

@dosubot dosubot bot added the community Related to langchain-community label Sep 19, 2024
@serena-ruan
Copy link
Contributor Author

@harupy Could you stamp this PR as well as you reviewed my last one :D Thanks!

@efriis efriis self-assigned this Sep 19, 2024
Signed-off-by: serena-ruan <[email protected]>
@efriis
Copy link
Member

efriis commented Sep 20, 2024

converting to draft - let me know when it's ready for a review on the langchain side!

As I've mentioned on a few other PRs, it would make sense to migrate the UC tools to the langchain-databricks repo.

@serena-ruan
Copy link
Contributor Author

converting to draft - let me know when it's ready for a review on the langchain side!

As I've mentioned on a few other PRs, it would make sense to migrate the UC tools to the langchain-databricks repo.

Thanks, we'll have some new implementation for langchain UC tools support, so want to keep the old one here :)

@serena-ruan serena-ruan marked this pull request as ready for review September 23, 2024 07:06
Copy link
Contributor

@BenWilson2 BenWilson2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@BenWilson2
Copy link
Contributor

@efriis Hey Erick! :) This one's ready for your review. Thanks!

@serena-ruan
Copy link
Contributor Author

Hi @efriis Could we merge this to catch the next release? Thanks!

@serena-ruan
Copy link
Contributor Author

Hi @baskaryan Could you help merge this PR? Thanks!

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Oct 9, 2024
@efriis
Copy link
Member

efriis commented Oct 9, 2024

Sorry for the delay!

@efriis efriis enabled auto-merge (squash) October 9, 2024 06:22
@efriis efriis merged commit a7c1ce2 into langchain-ai:master Oct 9, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Related to langchain-community lgtm PR looks good. Use to confirm that a PR is ready for merging. size:M This PR changes 30-99 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants