Skip to content

Update keyless-connections.md #472

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

Merged
merged 1 commit into from
Aug 11, 2025
Merged

Conversation

srishtigangulyabb
Copy link
Contributor

The correct audience for Azure Cognitive Search with RBAC is https://search.azure.com.

Old SDKs or legacy code might use .windows.net, but for Azure AD/RBAC, it’s .azure.com.

The correct audience for Azure Cognitive Search with RBAC is https://search.azure.com (not https://search.windows.net).

Old SDKs or legacy code might use .windows.net, but for Azure AD/RBAC, it’s .azure.com.
Copy link
Contributor

@srishtigangulyabb : Thanks for your contribution! The author(s) and reviewer(s) have been notified to review your proposed change.

Copy link
Contributor

Learn Build status updates of commit ac7675f:

✅ Validation status: passed

File Status Preview URL Details
articles/search/keyless-connections.md ✅Succeeded

For more details, please refer to the build report.

@v-dirichards
Copy link
Contributor

@HeidiSteen

Can you review the proposed changes?

Important: When the changes are ready for publication, adding a #sign-off comment is the best way to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator bot added the aq-pr-triaged C+L Pull Request Review Team label label Jul 2, 2025
@v-dirichards
Copy link
Contributor

@HeidiSteen Could you review this proposed update to your article and enter #sign-off in a comment if it's ready to merge?

Thanks!

Copy link
Contributor

@HeidiSteen HeidiSteen left a comment

Choose a reason for hiding this comment

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

Need more information on why you are proposing this edit.

@@ -184,7 +184,7 @@ from azure.search.documents import SearchClient
from azure.identity import DefaultAzureCredential, AzureAuthorityHosts

# Azure Public Cloud
audience = "https://search.windows.net"
audience = "[https://search.windows.net](https://search.azure.com)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @srishtigangulyabb, I don't understand this edit. This value is the scope ("In the context of Microsoft Entra, scopes refer to the permissions or access levels granted to external identities"), and it's a string, not a URL.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@srishtigangulyabb srishtigangulyabb Aug 11, 2025

Choose a reason for hiding this comment

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

The proposed change is from "https://search.windows.net" to "https://search.azure.com".

I have tried the code sample mentioned in the documentation but it gave me an error. When I learnt that the audience value should be "https://search.azure.com", I raised a request to update the documentation with the corrected value.

Copy link

This pull request has been inactive for at least 14 days. If you are finished with your changes, don't forget to sign off. See the contributor guide for instructions.
Get Help
Docs Support Teams Channel
Resolve Merge Conflict

@github-actions github-actions bot added the inactive This PR is inactive for more than 14 days label Jul 29, 2025
@ttorble
Copy link
Contributor

ttorble commented Aug 11, 2025

@srishtigangulyabb

Can you respond to the author's question?

image

@github-actions github-actions bot removed the inactive This PR is inactive for more than 14 days label Aug 11, 2025
Copy link
Contributor

@HeidiSteen HeidiSteen left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution. I took a closer look, and you are correct about the valid value. We'll merge your update, and then I'll go back and add a few more examples.

@HeidiSteen
Copy link
Contributor

#sign-off

@Court72 Court72 merged commit bf1511a into MicrosoftDocs:main Aug 11, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants