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.

Sign up for GitHub

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

Jump to bottom

langchain: included concurrency limit in RecursiveUrlLoader #15537

Closed
wants to merge 3 commits into from
Closed

langchain: included concurrency limit in RecursiveUrlLoader #15537

wants to merge 3 commits into from

Conversation

vincenzofanizza
Copy link

@vincenzofanizza vincenzofanizza commented Jan 4, 2024 •
edited
Loading

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jan 4, 2024
@vercel Vercel
Copy link

vercel bot commented Jan 4, 2024 •
edited
Loading

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Jan 12, 2024 5:24pm

@dosubot dosubot bot added Ɑ: doc loader Related to document loader module (not documentation) 🤖:improvement Medium size change to existing code to handle new use-cases labels Jan 4, 2024
baskaryan
baskaryan reviewed Jan 15, 2024
View reviewed changes
libs/community/langchain_community/document_loaders/recursive_url_loader.py
return []

# Wait if the concurrent requests limit is reached
if semaphore.locked():
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's this needed for, doesn't L241 already handle this?

Copy link
Author

Choose a reason for hiding this comment

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

I included it to allow the loader to complete as many requests as possible once the limit is reached before sending other requests. My idea was to insert a delay to avoid HTTP errors caused by an excessive number of simultaneous requests.

If you think that's redundant I can quickly update the code.

Copy link
Author

Choose a reason for hiding this comment

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

@baskaryan could you please tell me if I need to update the code?

@hwchase17 hwchase17 closed this Jan 30, 2024
@baskaryan baskaryan reopened this Jan 30, 2024
@ccurme ccurme added the community Related to langchain-community label Jun 18, 2024
@vincenzofanizza vincenzofanizza closed this by deleting the head repository Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baskaryan baskaryan baskaryan left review comments

Assignees

@nfcampos nfcampos

Labels
community Related to langchain-community Ɑ: doc loader Related to document loader module (not documentation) 🤖:improvement Medium size change to existing code to handle new use-cases size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vincenzofanizza @baskaryan @nfcampos @hwchase17 @ccurme