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

docs: small updates to astra vectorize docs #2497

Merged
merged 2 commits into from
Jul 4, 2024
Merged

Conversation

jordanrfrazier
Copy link
Collaborator

No description provided.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. documentation Improvements or additions to documentation labels Jul 2, 2024
name="provider_api_key",
display_name="Provider API Key",
info="An alternative to the Astra Authentication that passes an API key for the provider with each request to Astra DB. This may be used when Vectorize is configured for the collection, but no corresponding provider secret is stored within Astra's key management system.",
),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made this appear by default. The reasoning is that
a) This is the quickest path to testing out vectorize on a new collection
b) If the collection is already configured for vectorize, but the key was not stored in astra, this is (by my guess) the most common pattern for authentication.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the most common pattern is to store keys on astra, that's why I've set api_key_name in the front page.

If you go through the documentation the steps are to import the key in astra and refer to it when creating the collection

info="The name of the embeddings provider API key stored on Astra. If set, it will override the 'ProviderKey' in the authentication parameters.",
advanced=True,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a common case where the providerKey is set differently than the default?

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you mean? there's no default value. The value is defined by the user

Copy link
Collaborator Author

@jordanrfrazier jordanrfrazier Jul 3, 2024

Choose a reason for hiding this comment

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

I believe it does default just to providerKey, like so:

CollectionVectorServiceOptions(
           ...
            authentication={
                "providerKey": os.environ["OPENAI_API_KEY"],
           }
        )

But going through the UI flow, I see that you are correct in your other comment that the user-defined key name is required during creation. So I will undo these changes, and have Provider API Key Name shown by default, and Provider API Key in Advanced.

@@ -116,6 +116,7 @@ class AstraVectorStoreComponent(LCVectorStoreComponent):
name="embedding",
display_name="Embedding or Astra Vectorize",
input_types=["Embeddings", "dict"],
info="Allows either an embedding model or an Astra Vectorize configuration. If Astra Vectorize is already configured for the collection, this field is not required.",
Copy link
Contributor

@nicoloboschi nicoloboschi Jul 3, 2024

Choose a reason for hiding this comment

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

did you try what happens if you don't set it ?
I believe the AstraDBVectorStore requires one of those.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ugh, yeah. I guess that's something to figure out. It's required by langchain-astradb to have one or the other at the moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps we could loosen that constraint iff the setup mode is SetupMode.OFF?

@zzzming zzzming self-requested a review July 3, 2024 18:08
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 3, 2024
@ogabrielluiz ogabrielluiz enabled auto-merge (squash) July 4, 2024 18:24
@ogabrielluiz ogabrielluiz disabled auto-merge July 4, 2024 18:24
@ogabrielluiz ogabrielluiz enabled auto-merge (squash) July 4, 2024 18:25
@github-actions github-actions bot added documentation Improvements or additions to documentation and removed documentation Improvements or additions to documentation labels Jul 4, 2024
@ogabrielluiz ogabrielluiz merged commit a933139 into main Jul 4, 2024
31 checks passed
@ogabrielluiz ogabrielluiz deleted the vectorize-doc-updates branch July 4, 2024 18:25
ogabrielluiz added a commit to yaitec/langflow that referenced this pull request Jul 9, 2024
small updates to vectorize docs

Co-authored-by: Gabriel Luiz Freitas Almeida <[email protected]>
nicoloboschi pushed a commit to datastax/ragstack-ai-langflow that referenced this pull request Jul 10, 2024
small updates to vectorize docs

Co-authored-by: Gabriel Luiz Freitas Almeida <[email protected]>
(cherry picked from commit a933139)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants