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

[Integration]: Fireworks ai Integration #9131

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[Integration]: Fireworks ai Integration #9131

wants to merge 9 commits into from
+445 −0

Conversation

bala-ceg
Copy link
Contributor

Description

Created Fireworks.ai ML Integration.

Fixes #9043

Type of change

  • ⚡ New feature (non-breaking change which adds functionality)

Verification Process

To ensure the changes are working as expected:

  • Test Location: Please refer Testing Screenshots section
  • Verification Steps: Created the integration engine, verified all the data queries successfully mentioned in README.md

Additional Media:

  • I have attached a brief loom video or screenshots showcasing the new functionality or change.

Checklist:

  • My code follows the style guidelines(PEP 8) of MindsDB.
  • I have appropriately commented on my code, especially in complex areas.
  • Necessary documentation updates are either made or tracked in issues.
  • Relevant unit and integration tests are updated or added.

Testing Screenshots:

Screenshot 2024-04-24 at 11 16 01 PM Screenshot 2024-04-24 at 11 17 05 PM Screenshot 2024-04-24 at 11 17 55 PM Screenshot 2024-04-24 at 11 18 34 PM

@bala-ceg bala-ceg mentioned this pull request Apr 24, 2024
Open
@bala-ceg
Copy link
Contributor Author

@quest-bot loot #9043

@quest-bot quest-bot bot added the ⚔️ Quest Tracks quest-bot quests label Apr 24, 2024
@quest-bot quest-bot
Copy link

quest-bot bot commented Apr 24, 2024

Quest PR submitted! image Quest PR submitted!

@bala-ceg, you are attempting to solve the issue and loot this Quest. Will you be successful?

Questions? Check out the docs.

ZoranPandovski
ZoranPandovski requested changes Apr 25, 2024
View reviewed changes
Copy link
Member

@ZoranPandovski ZoranPandovski left a comment

Choose a reason for hiding this comment

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

Thanks. Please check the above comments. Also, let's include unit tests and implement the embeddings https://readme.fireworks.ai/docs/querying-embeddings-models

mindsdb/integrations/handlers/fireworks_handler/README.md Outdated Show resolved Hide resolved
mindsdb/integrations/handlers/fireworks_handler/config.py Outdated Show resolved Hide resolved
mindsdb/integrations/handlers/fireworks_handler/fireworks_handler.py Outdated
args = self.model_storage.json_get("args")


response = self.connection.ChatCompletion.create(
Copy link
Member

Choose a reason for hiding this comment

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

Let's add try-except to handle possible exceptions

mindsdb/integrations/handlers/fireworks_handler/fireworks_handler.py Outdated Show resolved Hide resolved
mindsdb/integrations/handlers/fireworks_handler/config.py Outdated Show resolved Hide resolved
@bala-ceg
Copy link
Contributor Author

@ZoranPandovski I have added embeddings and implemented all the review changes except adding tests.
Regarding tests, will it be suitable if just validate the query is being executed correctly rather validating the output

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZoranPandovski ZoranPandovski Awaiting requested review from ZoranPandovski

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
⚔️ Quest Tracks quest-bot quests
Projects
Tracking PRs
Status: to review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Integration]: Fireworks ai Integration
2 participants
@bala-ceg @ZoranPandovski