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

feat(BambooLLM): adding implementation for bamboollm updated version #1462

Merged
merged 4 commits into from
Dec 9, 2024

Conversation

ArslanSaleem
Copy link
Collaborator

@ArslanSaleem ArslanSaleem commented Dec 9, 2024

Important

Corrects environment variable names, updates request headers, and modifies BambooLLM implementation for improved functionality.

  • Environment Variables:
    • Corrected from PANDAAI_API_KEY to PANDASAI_API_KEY and PANDAAI_API_URL to PANDASAI_API_URL in __init__.py, base.py, and request.py.
  • Request Headers:
    • Changed Authorization header to x-authorization in make_request() in request.py.
  • BambooLLM Implementation:
    • Updated call() method in bamboo_llm.py to use /query endpoint and return response["answer"].

This description was created by Ellipsis for c07310e. It will automatically update as commits are pushed.

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Dec 9, 2024
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to ffae0d6 in 47 seconds

More details
  • Looked at 87 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. pandasai/dataframe/base.py:264
  • Draft comment:
    The error message should be updated to match the corrected environment variable names: PANDASAI_API_URL and PANDASAI_API_KEY.
  • Reason this comment was not posted:
    Marked as duplicate.
2. pandasai/helpers/request.py:106
  • Draft comment:
    The error message should be updated to match the corrected environment variable names: PANDASAI_API_URL and PANDASAI_API_KEY.
  • Reason this comment was not posted:
    Marked as duplicate.
3. pandasai/helpers/request.py:67
  • Draft comment:
    Ensure that the header key x-authorization is used consistently across the codebase for authorization.
  • Reason this comment was not posted:
    Comment did not seem useful.
4. pandasai/__init__.py:90
  • Draft comment:
                "Set PANDASAI_API_URL and PANDASAI_API_KEY in the environment to pull the dataset from the remote server"
  • Reason this comment was not posted:
    Marked as duplicate.
5. pandasai/dataframe/base.py:264
  • Draft comment:
                "Set PANDASAI_API_URL and PANDASAI_API_KEY in the environment to pull the dataset from the remote server"
  • Reason this comment was not posted:
    Marked as duplicate.
6. pandasai/helpers/request.py:106
  • Draft comment:
            "Set PANDASAI_API_URL and PANDASAI_API_KEY in the environment to push/pull the dataset to/from the remote server"
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_i3E9TQ4kVMSrR8d3


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

response = self._session.post(
"/query", json={"prompt": instruction.to_string()}
)
print(response)
Copy link

Choose a reason for hiding this comment

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

Remove the print(response) statement. Debugging statements should not be present in production code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

if not api_url or not api_key:
raise PandasAIApiKeyError(
"Set PANDAAI_API_URL and PANDAAI_API_KEY in environment to pull dataset from the remote server"
"Set PANDASAI_API_URL and PANDAAI_API_KEY in environment to pull dataset from the remote server"
Copy link

Choose a reason for hiding this comment

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

The error message should be updated to match the corrected environment variable names: PANDASAI_API_URL and PANDASAI_API_KEY.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 1c7acb1 in 27 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. pandasai/llm/bamboo_llm.py:21
  • Draft comment:
    Consider adding error handling for the _session.post call to manage potential exceptions and ensure the method returns a valid response or error message.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_aQ4W9zxkSTZzonrE


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on c07310e in 33 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. pandasai/__init__.py:90
  • Draft comment:
    The environment variable names have been correctly updated to PANDASAI_API_URL and PANDASAI_API_KEY as per the PR description.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR description mentions changing the environment variable names, and this change is correctly reflected in the code.

Workflow ID: wflow_JJFY58kIKAYvxUyq


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

if not api_url or not api_key:
raise PandasAIApiKeyError(
"Set PANDAAI_API_URL and PANDAAI_API_KEY in environment to pull dataset from the remote server"
"Set PANDASAI_API_URL and PANDASAI_API_KEY in environment to pull dataset from the remote server"
Copy link

Choose a reason for hiding this comment

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

The error message can be improved for clarity and conciseness:

Suggested change
"Set PANDASAI_API_URL and PANDASAI_API_KEY in environment to pull dataset from the remote server"
"Please set the PANDASAI_API_URL and PANDASAI_API_KEY environment variables to pull the dataset from the remote server."

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@gventuri gventuri merged commit b12cb49 into release/v3 Dec 9, 2024
0 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants