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

langchain[patch],community[patch]: add in-memory option for unstructured loader #5581

Closed

Conversation

andrewdoro
Copy link
Contributor

@andrewdoro andrewdoro commented May 28, 2024

Fixes #1618

Add support for passing buffer to unstructured loader.

This covers cases when fetching a file externally using serverless functions. Without this change we always have to write a new file before passing the path to the loader. This is an anti pattern for serverless environments, such as Vercel.
https://github.com/orgs/vercel/discussions/241#discussioncomment-363002

Old implementation was already loading the file in memory.

Fixes # (issue)
twitter @andrewdorobantu

Copy link

vercel bot commented May 28, 2024

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

Name Status Preview Comments Updated (UTC)
langchainjs-api-refs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 9, 2024 7:06am
langchainjs-docs ✅ Ready (Inspect) Visit Preview Jun 9, 2024 7:06am

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 28, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. auto:improvement Medium size change to existing code to handle new use-cases and removed size:M This PR changes 30-99 lines, ignoring generated files. labels May 28, 2024
@@ -29,6 +30,34 @@ test.skip("Test Unstructured base loader", async () => {
}
Copy link

Choose a reason for hiding this comment

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

Hey team, just a heads up that I've flagged a change in the PR related to accessing an environment variable using process.env. This is for your review to ensure proper handling of environment variables. Keep up the great work!

Copy link
Contributor Author

@andrewdoro andrewdoro May 28, 2024

Choose a reason for hiding this comment

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

that's right, I've run the tests locally using a docker instance of unstructured

@@ -175,19 +184,30 @@ export class UnstructuredLoader extends BaseDocumentLoader {
private maxCharacters?: number;

constructor(
filePathOrLegacyApiUrl: string,
filePathOrLegacyApiUrlOrMemoryBuffer:
Copy link
Collaborator

Choose a reason for hiding this comment

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

We reeealy need to clean this up 😬

@jacoblee93
Copy link
Collaborator

Looks reasonable, thank you!

@jacoblee93
Copy link
Collaborator

Can't push to your branch - this lint rule is pretty silly, so you can either fix it or just add "prefer-destructuring": 0, to langchain/eslintrc.cjs and libs/langchain-community/eslintrc.cjs.

@andrewdoro
Copy link
Contributor Author

@jacoblee93 fixed the formatting

@jacoblee93
Copy link
Collaborator

Thank you!

@jacoblee93
Copy link
Collaborator

Merged in #5726

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:improvement Medium size change to existing code to handle new use-cases size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow unstructured loaders to accept in-memory text
2 participants