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(go/plugins/firebase): add retriever, init methods #1470

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cabljac
Copy link
Contributor

@cabljac cabljac commented Dec 9, 2024

This PR adds retrievers to the Go firebase plugin.

Should resolve #711

  • Consolidated state management into a single state struct, similar to the Google AI plugin
  • Added thread-safety with mutex
  • Added support for retrievers in the state management

Checklist (if applicable):

if app == nil {
newApp, err := firebase.NewApp(ctx, nil)
if state.initted {
log.Println("firebase.Init: plugin already initialized, returning without reinitializing")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We discussed this before, but I don't remember the outcome of the discussion -- are we sure we want to no-op when it's possible there is a different config being passed in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if this is the right behavior or if it should error on subsequent calls. Generally this no-op behavior is only deterministic if you are not passing a configuration that you are expecting to take effect. Here, a no-op means you don't know if the initted config matches what you're expecting.

yup you're right, sorry it got lost across versions of this PR, will look to change

}

// Retrievers returns the cached list of retrievers.
func Retrievers(ctx context.Context) ([]ai.Retriever, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't need ctx here or above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[Go] Add retriever plugin for firestore vector store
2 participants