-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Azure AI Search settings throws, failing to resolve service #14984
Comments
@bleroy check your logs please. I am guessing you did not provide your settings. |
What settings should I have provided? I'm very confused, I was trying to get to the settings screen to configure the feature, and now you're telling me I need to provide settings? |
Is search services, we follow similar logic where we don't register service if the configuration does not exists. Maybe this is not the best idea. I'll check. Maybe I'll drop the return statement if register all service anyway |
I'm expecting to be able to configure my search AI service from the settings screen, not from some text file (which I'm assuming couldn't even work at all if I'm on a hosted Orchard instance). |
An exception stack trace and logs also seem like a poor discovery mechanism. |
@bleroy sorry I was not clear earlier. You'll need to update your appsettings.json file
https://docs.orchardcore.net/en/latest/docs/reference/modules/AzureAISearch/ Does this solve the problem? |
Using configuration here allows you to setup the secrets in ENV, have different values per environment (App Service), and even support service connectors auth (does AI Search support it?) |
Yes, it does support managed identities, in which case there may be no secret at all to specify anywhere in Orchard. My other point about discoverability stands. |
managed identities != service connectors (was new to me, but I find it awesome) |
When using Managed Service or app connector, don't we have to provide some sort of secret in OC to connect with? Maybe what we need is a new module in OC for authenticating in Azure via (Managed Services, app connectors or others) This way, any Azure module we have like Azure AI Search can specify which type of connection to use "appsettings (default)", managed service, connector app or anything else. Meanwhile, I'll change the Azure AI Search implementation to not throw exception if the configuration does not exist. Instead, we'll show warning on the UI |
@bleroy I am looking for example on how to connect the to Azure AI Search instance using Managed Identity or App Connector. But, I am not finding much. Either way, I am guessing you'll need to provide some sort of settings to initialize the client. For example,
Do you agree that this info need to be in the Are you able to point me to the correct docs (example if available), on how to configure the Azure Client using connection string? |
Sure, this should give you a good view of the ways to authenticate to Search: https://learn.microsoft.com/en-us/azure/search/search-security-overview Actually, no, I don't agree that info needs to be in appsettings. I don't see any reason why it couldn't be stored in the Orchard database, but I'm not up-to-date on Orchard Core settings, how it manages secrets, if it supports Key Vault, etc. @sebastienros must have some opinions and knowledge about that that I lack currently. |
@bleroy There are two different consumption scenario for this (and probably the other search) modules in OC which are as follow:
Since scenario 1 is the only option we have today. You'll need to edit the
I can defiantly add support for scenario 2 (which seems to be what you are after). Better yet, we should have a way to allow case 1, case 2 or both cases. In either way, I think we need to first provide support for other types of authentication into Azure Search AI. Currently, we configure the Azure Clients here https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore/OrchardCore.Search.AzureAI.Core/Extensions/ServiceCollectionExtensions.cs#L42-L45 For the above to work, the configuration section will need to look like this
Question If we were to connect to the Azure service using managed-identity or Microsoft Entra, how would the connection need to look like so that the client is configured using managed-identity instead of the Key? Knowing this will then allow me to add support for multi authentication types. Then add support for both consumption scenarios and allow the admin to specify which behavior they want to enable. |
Thanks for the additional details. 1 would imply the SaaS owner also owns the search service, gets billed for it, and transfers the cost to their customer, the end user. Billing for a search service can be quite hairy. As a SaaS business owner, I would see this as quite complex to setup, including in terms of accounting. I'd rather give the user the option to bring their own service, so they're the ones dealing with the cost. Also thanks for the description of the settings that are currently necessary. It would be great to have that info as part of the error message or in docs. Here's some more docs that have info about principals and auth when accessing the service through the API: https://learn.microsoft.com/en-us/azure/search/search-manage-rest |
I'll update the docs along with the environment variable once I am done implementing scenario 2. The docs link you sent me does not explain should the
instead of
I just want to know how to configure the Azure client using C# with using manage identity. |
Well, the stuff you put into appsettings is up to you, it's just one way to store configuration for the client library. It's your code that's going to read those values and use them to configure the client. You're going to create a client with an instance of |
Do I need to do anything special to cause the index to get populated? I setup everything in Search / Settings after I created my index. I verified in the portal that the index was created and seems to have all the right fields, but no matter what I do (including reset and rebuild from Orchard), the index is still 0 documents and 0 bytes from the portal, and no query returns any document. Yet, from the Orchard admin, it says there's one item in the index. My logs contain the following suspicious stack trace:
|
@bleroy That error in interesting, maybe multiple parts are trying to compose the same field. I did not encounter that error on my side. I'll try to reproduce it. You may need to check your content definition to ensure that the parts that you want to index are enabled. If you edit the content part, you should have a settings in there to include the part in the Azure AI search index. |
OK, so I got the latest version, reset all my data and started over. I'm able to create an index, and I see it's been populated from the Azure portal where I can query it and get results. When I try to do that through a search widget however, I can't seem to get any results. I'm still getting a null ref on the settings page if I fail to provide the azure search service connection information in appsettings though, so that part still doesn't work for me. |
@bleroy are you using preview nuget packages, using main branch or the |
Locally compiled tip of main. I thought this was merged into main. I'll try that branch. |
I did not merge the recent changes. Yes please try out |
OK, so yes, I was able to put the settings in the admin, great. However, still no search results. Nothing in logs. |
@bleroy Did you rebuild your indexes after changing branch? If you view the result in Azure portal when you search, which field are the results are found in? Are you able to export your content type definitions, and your content items so I can try to replicate your issue? To confirm, you can set the default search provider as Did you configure which index to search with in by default? Here is the content type definition of my When I search for |
@bleroy also we have our weekly steering meeting in 10 mins. If you are able to, you can join and we can troubleshoot your issue 1 on 1 and ensure to get to the bottom of it. If you can join, orchardcore.net/meet |
I did rebuild everything after the branch change, and also deleted the previous index from Azure. I started from scratch using the blog recipe and indexed the blog type. Here's the result of a * search in the Azure portal: {
"@odata.context": "https://beleroy-cognitive-search.search.windows.net/indexes('default-orchardindex')/$metadata#docs(*)",
"value": [
{
"@search.score": 1,
"ContentItemId": "4emy98p5yd9zt0g5xarvg1j1n7",
"ContentItemVersionId": "4xsdrntt76mzjvhpzk1gpcn551",
"Content__ContentItem__Owner": "4hxtc7qrq7bdy1pv10q8eb6kxa",
"Content__ContentItem__DisplayText__Analyzed": [
"Man must explore, and this is exploration at its greatest"
],
"Content__ContentItem__FullText": [
"Man must explore, and this is exploration at its greatest <p>Never in all their history have men been able truly to conceive of the world as one: a single sphere, a globe, having the qualities of a globe, a round earth in which all the directions eventually meet, in which there is no center because every point, or none, is center — an equal earth which all men occupy as equals. The airman's earth, if free men make it, will be truly round: a globe in practice, not in theory.</p>\n<p>Science cuts two ways, of course; its products can be used for both good and evil. But there's no turning back from science. The early warnings about technological dangers also come from science.</p>\n<p>What was most significant about the lunar voyage was not that man set foot on the Moon but that they set eye on the earth.</p>\n<p>A Chinese tale tells of some men sent to harm a young girl who, upon seeing her beauty, become her protectors rather than her violators. That's how I felt seeing the Earth for the first time. I could not help but love and cherish her.</p>\n<p>For those who have seen the Earth from space, and for the hundreds and perhaps thousands more who will, the experience most certainly changes your perspective. The things that we share in our world are far more valuable than those which divide us.</p>\n "
],
"Content__BodyAspect__Body": [
"<p>Never in all their history have men been able truly to conceive of the world as one: a single sphere, a globe, having the qualities of a globe, a round earth in which all the directions eventually meet, in which there is no center because every point, or none, is center — an equal earth which all men occupy as equals. The airman's earth, if free men make it, will be truly round: a globe in practice, not in theory.</p>\n<p>Science cuts two ways, of course; its products can be used for both good and evil. But there's no turning back from science. The early warnings about technological dangers also come from science.</p>\n<p>What was most significant about the lunar voyage was not that man set foot on the Moon but that they set eye on the earth.</p>\n<p>A Chinese tale tells of some men sent to harm a young girl who, upon seeing her beauty, become her protectors rather than her violators. That's how I felt seeing the Earth for the first time. I could not help but love and cherish her.</p>\n<p>For those who have seen the Earth from space, and for the hundreds and perhaps thousands more who will, the experience most certainly changes your perspective. The things that we share in our world are far more valuable than those which divide us.</p>\n"
],
"Content__ContentItem__DisplayText__keyword": "Man must explore, and this is exploration at its greatest",
"Content__ContentItem__DisplayText__Normalized": "man must explore, and this is exploration at its greatest",
"Content__ContentItem__ContentType": "BlogPost",
"Content__ContentItem__CreatedUtc": "2024-01-09T19:19:21.021Z",
"Content__ContentItem__Latest": true,
"Content__ContentItem__Author": "admin",
"Content__ContentItem__ModifiedUtc": "2024-01-09T19:19:21.024Z",
"Content__ContentItem__Published": true,
"Content__ContentItem__PublishedUtc": "2024-01-09T19:19:21.045Z"
}
]
} I have a conflict with the Tuesday meetings, but that meeting ended early, so I just joined. |
@bleroy I fixed the issue. Can you please pull the latest and try again? Make sure you set the Search settings. |
No luck. Still no search results. I took the latest and recompiled. I didn't recreate the site or the index. Do I need to? |
@bleroy I can't reproduce the issue at all. I did made some changes and wonder if you can pull the latest and test it one more time? If your issue that you do NOT see a search form? If you do see the search form, can you please show me a screenshot of the search settings? and |
@sebastienros @bleroy I added Managed Identity authentication settings with an optional way of providing identity client id. |
I was able to create a new site using the blog recipe and everything worked for me. Below, you'll see a screencast of the exact steps I took and everything worked. Maybe for your demo today, you should start with a new tenant using the blog theme. However, there is a bug in the
You'll notices in the screencast below, I enabled |
OK, so the search results were still not there, until I enabled Lucene like you did. I guess Lucene should be a dependency of the feature, at least for now... |
@bleroy I rather not add this kind of dependency. If you switch your theme using TheDefault theme it works. So the issue must be in TheBlogTheme. I created an issue #15033 so we can fix it So did everything worked for you now to 100% demo? |
Technically, on the Orchard side, yes, the demo works 100%. Thanks a lot for all your efforts. Teams kept crashing every couple of minutes though so we had to postpone. Sad. |
Describe the bug
Settings for the new Azure AI Search are not accessible after setting up a simple site from scratch.
cc @MikeAlhayek
To Reproduce
Create a new Agency site with everything default
Activate Azure AI Search feature as well as Indexing and Search
Select Search / Settings from the admin menu
Expected behavior
Search screen to appear
Actual
Getting an exception:
Screenshots
If applicable, add screenshots to help explain your problem.
The text was updated successfully, but these errors were encountered: