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[minor]: Taskade Project Loader #5927

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

yeouchien
Copy link
Contributor

This PR adds Taskade Project Loader to community document loader.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 28, 2024
Copy link

vercel bot commented Jun 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 Jul 2, 2024 8:55pm
langchainjs-docs ✅ Ready (Inspect) Visit Preview Jul 2, 2024 8:55pm

@dosubot dosubot bot added the auto:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features label Jun 28, 2024
@@ -0,0 +1,9 @@
import { TaskadeProjectLoader } from "@langchain/community/document_loaders/web/taskade";
Copy link

Choose a reason for hiding this comment

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

Hey there! I noticed that this PR adds a new external HTTP request using fetch or axios in the TaskadeProjectLoader. I've flagged this for your review to ensure it aligns with the project's requirements. Let me know if you have any questions!

@@ -0,0 +1,9 @@
import { TaskadeProjectLoader } from "@langchain/community/document_loaders/web/taskade";
Copy link

Choose a reason for hiding this comment

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

Hey there! I noticed that the recent change in the taskade.ts file suggests accessing an environment variable for the personalAccessToken. I've flagged this for your review to ensure it aligns with our best practices. Let me know if you have any questions!

@@ -0,0 +1,13 @@
/* eslint-disable no-process-env */
Copy link

Choose a reason for hiding this comment

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

Hey there! I noticed that this PR adds a test case that explicitly accesses environment variables via process.env. I've flagged this for your review to ensure proper handling of environment variables. Let me know if you have any questions or need further assistance!

@@ -0,0 +1,125 @@
import { getEnvironmentVariable } from "@langchain/core/utils/env";
Copy link

Choose a reason for hiding this comment

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

Hey there! I've reviewed the code changes, and it looks like the TaskadeProjectLoader class introduces a new external HTTP request using the fetch API to fetch project tasks from the Taskade API. I've flagged this change for your review to ensure it aligns with the project's requirements. Let me know if you have any questions or need further clarification!

@@ -0,0 +1,125 @@
import { getEnvironmentVariable } from "@langchain/core/utils/env";
Copy link

Choose a reason for hiding this comment

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

Hey team, I've flagged a change in the PR that explicitly accesses an environment variable using the getEnvironmentVariable function. This is to ensure the maintainers review and validate the handling of environment variables in the code.

@johnxie
Copy link

johnxie commented Jun 29, 2024

Thanks, @yeouchien—please do check the comments above.

Hey everyone, John from Taskade here! We're excited about some upcoming updates related to our multi-agents and use cases. You can catch the latest at https://www.taskade.com/blog/updates/.

@hwchase17, we're looking forward to contributing more and figuring out the best ways to make our Custom AI Agents work with https://github.com/langchain-ai/opengpts and offer perhaps enhanced tool compatibility.

Open to feedback and suggestions. Thanks again, everyone!

@jacoblee93
Copy link
Collaborator

Thank you! LGTM

@jacoblee93 jacoblee93 added the lgtm PRs that are ready to be merged as-is label Jul 2, 2024
@johnxie
Copy link

johnxie commented Jul 3, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features lgtm PRs that are ready to be merged as-is size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants