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

Documentation for the PostgreSQL vector store connector #204

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

lossyrob
Copy link

This PR adds the Postgres code samples to code-samples.md, expands on some of the dotnet docs, and adds python docs for the Postgres connector.

Includes some expanded docs around dotnet and adds docs for the python
Copy link
Contributor

Learn Build status updates of commit 1c05ab7:

⚠️ Validation status: warnings

File Status Preview URL Details
semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md ⚠️Warning Details
semantic-kernel/concepts/vector-store-connectors/code-samples.md ✅Succeeded

semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md

  • Line 172, Column 86: [Warning: hard-coded-locale - See documentation] Link 'https://learn.microsoft.com/en-us/azure/postgresql/flexible-server/concepts-azure-ad-authentication' contains locale code 'en-us'. For localizability, remove 'en-us' from links to most Microsoft sites.
  • Line 305, Column 58: [Warning: hard-coded-locale - See documentation] Link 'https://learn.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential?view=azure-dotnet' contains locale code 'en-us'. For localizability, remove 'en-us' from links to most Microsoft sites.
  • Line 394, Column 86: [Warning: hard-coded-locale - See documentation] Link 'https://learn.microsoft.com/en-us/azure/postgresql/flexible-server/concepts-azure-ad-authentication' contains locale code 'en-us'. For localizability, remove 'en-us' from links to most Microsoft sites.
  • Line 492, Column 55: [Warning: hard-coded-locale - See documentation] Link 'https://learn.microsoft.com/en-us/python/api/azure-identity/azure.identity.defaultazurecredential?view=azure-python' contains locale code 'en-us'. For localizability, remove 'en-us' from links to most Microsoft sites.
  • Line 172, Column 86: [Suggestion: docs-link-absolute - See documentation] Absolute link 'https://learn.microsoft.com/en-us/azure/postgresql/flexible-server/concepts-azure-ad-authentication' will be broken in isolated environments. Replace with a relative link.
  • Line 305, Column 58: [Suggestion: docs-link-absolute - See documentation] Absolute link 'https://learn.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential?view=azure-dotnet' will be broken in isolated environments. Replace with a relative link.
  • Line 305, Column 58: [Suggestion: preserve-view-not-set - See documentation] You've pinned this link to a specific version of content with the view parameter. It's recommended not to pin a version unless that version is A) not the default view and B) the context is about that version specifically. To proceed with pinning a version add the &preserve-view=true to the URL. Otherwise, remove the view parameter. URL: https://learn.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential?view=azure-dotnet
  • Line 394, Column 86: [Suggestion: docs-link-absolute - See documentation] Absolute link 'https://learn.microsoft.com/en-us/azure/postgresql/flexible-server/concepts-azure-ad-authentication' will be broken in isolated environments. Replace with a relative link.
  • Line 492, Column 55: [Suggestion: docs-link-absolute - See documentation] Absolute link 'https://learn.microsoft.com/en-us/python/api/azure-identity/azure.identity.defaultazurecredential?view=azure-python' will be broken in isolated environments. Replace with a relative link.
  • Line 492, Column 55: [Suggestion: preserve-view-not-set - See documentation] You've pinned this link to a specific version of content with the view parameter. It's recommended not to pin a version unless that version is A) not the default view and B) the context is about that version specifically. To proceed with pinning a version add the &preserve-view=true to the URL. Otherwise, remove the view parameter. URL: https://learn.microsoft.com/en-us/python/api/azure-identity/azure.identity.defaultazurecredential?view=azure-python

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

Copy link
Contributor

Learn Build status updates of commit a05198b:

✅ Validation status: passed

File Status Preview URL Details
semantic-kernel/concepts/vector-store-connectors/code-samples.md ✅Succeeded
semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@lossyrob
Copy link
Author

@westey-m now that both the pythion and dotnet versions are released for PG vector search, it would be great to get the docs published as well. What's the process for review and publishing here? Thanks!

Copy link
Contributor

@westey-m westey-m left a comment

Choose a reason for hiding this comment

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

Please take a look at the comments I added.

logger = logging.getLogger(__name__)


async def get_entra_token_async(credential: AsyncTokenCredential) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

in semantic_kernel/utils/authentication/entra_id_authentication.py we have prebuilt function for this, just pass the endpoint into that!

Copy link
Author

Choose a reason for hiding this comment

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

That function is helpful, but there's not an async version and doesn't take in a credential as an option so always uses DefaultAzureCredential...passing in a credential is useful e.g. when you are operating in a service pod on AKS that has a workload identity, and you don't want to iterate through the possible credential paths because you only want a single credential mechanism.

I could contribute a PR that adds the credential argument and async version if you think that would be useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

that would be very useful, I'm very surprised there is no async in that one... and I like the idea of passing specific creds!

Copy link
Author

Choose a reason for hiding this comment

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

Ok, put that on the TODO list. Looks like there's implications for the classes that use it, e.g. allowing credentials to be specificed in AzureOpenAIConfigBase.Do you think that should be blocking to this PR? It would be great to get this into the docs now that the postgres vectorstore feature is published. I can update the docs once the utils change gets merged and released.

Copy link
Contributor

Learn Build status updates of commit 0c40d8b:

✅ Validation status: passed

File Status Preview URL Details
semantic-kernel/concepts/vector-store-connectors/code-samples.md ✅Succeeded
semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

Learn Build status updates of commit 1c3f6e0:

✅ Validation status: passed

File Status Preview URL Details
semantic-kernel/concepts/vector-store-connectors/code-samples.md ✅Succeeded
semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

Learn Build status updates of commit 9c2a63f:

✅ Validation status: passed

File Status Preview URL Details
semantic-kernel/concepts/vector-store-connectors/code-samples.md ✅Succeeded
semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@lossyrob
Copy link
Author

lossyrob commented Mar 11, 2025

@westey-m , @eavanvalkenburg - Thanks for the review! Apologies for the low quality code examples; guess I've been leaning on the type checker harder than I thought :-) I put the code examples into project code files to make sure the types were correct, and addressed all comments. I have a PR to make against semantic-kernel to add the PRIMARY KEY constraint and potentially to add functionality to the semantic_kernel.utils.authentication.entra_id_authentication methods; I left those conversations unresolved for your input.

(Edit: Turns out I was implementing the PRIMARY KEY constraint correctly, just differently than I expected. updated docs to reflect this).

Copy link
Contributor

Learn Build status updates of commit a7a7477:

✅ Validation status: passed

File Status Preview URL Details
semantic-kernel/concepts/vector-store-connectors/code-samples.md ✅Succeeded
semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@lossyrob lossyrob requested a review from westey-m March 14, 2025 21:42
Copy link
Contributor

Learn Build status updates of commit a2ee2a8:

✅ Validation status: passed

File Status Preview URL Details
semantic-kernel/concepts/vector-store-connectors/code-samples.md ✅Succeeded
semantic-kernel/concepts/vector-store-connectors/out-of-the-box-connectors/postgres-connector.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

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.

3 participants