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

Fix web_base.py #6519

Merged
merged 7 commits into from
Jul 5, 2023
Merged

Fix web_base.py #6519

merged 7 commits into from
Jul 5, 2023

Conversation

zomchak-code
Copy link
Contributor

Fix for bug in SitemapLoader

aiohttp get does not accept verify argument, and currently throws error, so SitemapLoader is not working

This PR fixes it by removing verify param for get function call

Fixes #6107

Who can review?

Tag maintainers/contributors who might be interested:

@eyurtsev

@vercel
Copy link

vercel bot commented Jun 21, 2023 •
edited
Loading

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 Jul 3, 2023 10:12pm

@vercel vercel bot temporarily deployed to Preview June 21, 2023 06:58 Inactive
arunchinnachamy
arunchinnachamy suggested changes Jun 22, 2023
View reviewed changes
Copy link

@arunchinnachamy arunchinnachamy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

the parameter needs to be removed from Line No. 188 as well in the function scrape.

https://github.com/hwchase17/langchain/pull/6519/files/bf66b441be6496cf1d4227bd54d2b941433f5580#diff-8d822032870450954e168387e16257457d811a2539a5e94fa9317865f88dde71L188

@arunchinnachamy arunchinnachamy mentioned this pull request Jun 22, 2023
Closed
14 tasks
@vercel vercel bot temporarily deployed to Preview June 27, 2023 17:39 Inactive
@zomchak-code
Copy link
Contributor Author

@arunchinnachamy

the parameter needs to be removed from Line No. 188 as well in the function scrape.

Line 188 is using self.session which is a requests session object
I've refactored the code and moved this param setting in the constructor

@vercel vercel bot temporarily deployed to Preview June 27, 2023 17:48 Inactive
zomchak-code
zomchak-code commented Jun 27, 2023
View reviewed changes
Copy link
Contributor Author

@zomchak-code zomchak-code left a comment

Choose a reason for hiding this comment

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

@dev2049 @pandemosth @arunchinnachamy Comments addressed

langchain/document_loaders/web_base.py Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview June 28, 2023 13:57 Inactive
@Yevgnen Yevgnen mentioned this pull request Jul 3, 2023
Merged
@zomchak-code
Copy link
Contributor Author

@rlancemartin, @eyurtsev
Let me know what I can do to help merge this

@vercel vercel bot temporarily deployed to Preview July 3, 2023 22:12 Inactive
@zomchak-code
Copy link
Contributor Author

@rlancemartin, @eyurtsev
I fixed the linting error

@zomchak-code
Copy link
Contributor Author

@rlancemartin, @eyurtsev
Sorry for spamming, but this is a small fix for important bug. I currently can not use langchain in my workflow because of it.
Just let me know what can I do to get this merged faster

@rlancemartin rlancemartin self-assigned this Jul 5, 2023
@rlancemartin
Copy link
Collaborator

@rlancemartin, @eyurtsev Sorry for spamming, but this is a small fix for important bug. I currently can not use langchain in my workflow because of it. Just let me know what can I do to get this merged faster

Thanks! Running tests now and will merge once it passes.

@rlancemartin rlancemartin merged commit 8afc8e6 into langchain-ai:master Jul 5, 2023
hwchase17 added a commit that referenced this pull request Jul 6, 2023
@Yevgnen @hwchase17
Add concurrency to GitbookLoader (#7069)
930e319 
- Description: Fetch all pages concurrently.
- Dependencies: `scrape_all` -> `fetch_all` -> `_fetch_with_rate_limit`
-> `_fetch` (might be broken currently:
#6519)
  - Tag maintainer: @rlancemartin, @eyurtsev

---------

Co-authored-by: Harrison Chase <hw.chase.17@gmail.com>
@unified-repo-staging unified-repo-staging bot mentioned this pull request Jul 14, 2023
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pandemosth pandemosth Awaiting requested review from pandemosth

@arunchinnachamy arunchinnachamy Awaiting requested review from arunchinnachamy

@dev2049 dev2049 Awaiting requested review from dev2049

Assignees

@rlancemartin rlancemartin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zomchak-code @rlancemartin @arunchinnachamy @pandemosth @dev2049