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]: Basic Fireworks function calling notebook + embeddings code #4447

Merged

Conversation

benjibc
Copy link
Contributor

@benjibc benjibc commented Feb 17, 2024

  • Basic Dino notebook with LangChain tools and Fireworks function calling model
  • Code for Fireworks Embeddings

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

vercel bot commented Feb 17, 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 Feb 21, 2024 4:25am
langchainjs-docs ✅ Ready (Inspect) Visit Preview Feb 21, 2024 4:25am

@dosubot dosubot bot added the auto:documentation Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder label Feb 17, 2024
@@ -0,0 +1,152 @@
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 noticed that the recent changes added a new HTTP request using fetch to the Fireworks AI API in the embeddingWithRetry method. I've flagged this 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,152 @@
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 addition of environment variable access via getEnvironmentVariable is flagged for review. This is important for maintainers to ensure proper handling of sensitive information. Great work, and let me know if you have any questions!

@benjibc benjibc force-pushed the basic_langchain_fw_fc_and_embeddings branch from ab8f5c6 to c4cec08 Compare February 17, 2024 07:17
@jacoblee93 jacoblee93 changed the title Basic Fireworks function calling notebook + embeddings code community[minor]: Basic Fireworks function calling notebook + embeddings code Feb 20, 2024
@jacoblee93
Copy link
Collaborator

Code looks good! I can't view the notebook in the diff, but will try locally later today.

We'll want to add a page under the integrations docs as well, but I can do that later as well. Thanks for your patience!

@jacoblee93 jacoblee93 self-assigned this Feb 20, 2024
@benjibc
Copy link
Contributor Author

benjibc commented Feb 20, 2024

@jacoblee93 I think this link works: https://github.com/langchain-ai/langchainjs/blob/c4cec0877411f2b3bf6270e67be4e02fa6e54f9d/cookbook/function_calling_fireworks.ipynb .

For where to add the docs, perfectly happy to do the work, is this the doc you have in mind? https://github.com/langchain-ai/langchainjs/tree/main/libs/langchain-community ? If not can you provide me some pointers?

@jacoblee93 jacoblee93 added the lgtm PRs that are ready to be merged as-is label Feb 21, 2024
@jacoblee93
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:documentation Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder 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.

2 participants