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

fix: Correct order of checks for environment variable keys, enabling Ollama defaults to work #1591

Closed

Conversation

auphof
Copy link

@auphof auphof commented Nov 13, 2024

This addresses #1584

  • Moved the check for unnacepted_attributes to ensure it is evaluated after confirming the presence of key_name in the environment variable.
  • This change improves the clarity and correctness of the environment variable handling logic, allowing the Ollama default env API_BASE to be handled in the Agent setup

- Moved the check for `unnacepted_attributes` to ensure it is evaluated after confirming the presence of `key_name` in the environment variable.
- This change improves the clarity and correctness of the environment variable handling logic.
@auphof
Copy link
Author

auphof commented Nov 13, 2024

I am not familiar with the testing of CrewAI, particularly as to where the appropriate test should be created , I am happy to create if someone could please point me in the correct direction

@auphof
Copy link
Author

auphof commented Nov 13, 2024

@joaomdmoura @bhancockio as you both worked in this area over the last week, please review or pass on appropriately the review of this PR #1591 together with #1589. Happy to assist where ever, Thank you for an awesome framework and that was a great interview with Matthew Berman https://x.com/joaomdmoura/status/1856384282475774441 👏👏👏

@bhancockio
Copy link
Collaborator

Hey @auphof!

Thank you for creating this PR and pointing out the error.

The error is now fixed. Please keep us posted if you find any other errors or issues 😁

Closed by #1638

@bhancockio bhancockio closed this Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants