yoheinakajima / babyagi

MIT License
19.73k stars 2.58k forks source link

Add support for weaviate #254

Closed hsm207 closed 1 year ago

hsm207 commented 1 year ago

Fixes #239

hsm207 commented 1 year ago

@Ryangr0 @francip how's this PR looking? We just need to prepare some docs so it would be great if you could review the code in the mean time.

francip commented 1 year ago

Looks good for the most part. The only nitpick I have is the fallback logic. Here's what I'd like to see (but we can do as follow up)

  1. Check if Weaviate env variable there
  2. If Yes, check if Weaviate is installed and can be loaded
  3. If Yes, everything's peachy
  4. If Weaviate env variable not there or Weaviate not installed, check Pinecone (same algo as above)
  5. If Pinecone fails, use Chroma.

With the current implementation, if Weaviate is turned on, but fails, it directly goes to Chroma. Not a big issue, as allegedly people that try to configure Weaviate don't care about Pinecone... :-)

hsm207 commented 1 year ago
  • Check if Weaviate env variable there
  • If Yes, check if Weaviate is installed and can be loaded
  • If Yes, everything's peachy
  • If Weaviate env variable not there or Weaviate not installed, check Pinecone (same algo as above)
  • If Pinecone fails, use Chroma.

@francip I've refactored the fallback logic based on your feedback. All good?

francip commented 1 year ago

Looks good. Let's merge it in... (It's still a draft rn, so I can't :-) )

francip commented 1 year ago

Are we landing this or what? :-p

hsm207 commented 1 year ago

it will be ready to merge in a couple of days. @zainhas is making some final touches on the docs 😀

hsm207 commented 1 year ago

@francip we're done with the docs

francip commented 1 year ago

Thanks! Merged.