Update Search code based on AutoRest spec version e6fa7db9 #20078
Conversation
This produces a successful build
Mohit-Chakraborty
requested review from
KrzysztofCwalina,
arv100kri,
bleroy,
brjohnstmsft,
heaths and
tg-msft
as code owners
ghost
added
the
|
cc: @tg-msft |
sdk/search/Azure.Search.Documents/src/Generated/Models/AnswerResult.Serialization.cs
Show resolved
Hide resolved
heaths
left a comment
heaths
left a comment
There was a problem hiding this comment.
A few comments. Mainly I want to see the ODataType params and properties cleaned up given how we do it for other discriminated types in the SDK. Other languages should be similar, i.e. the OData stuff generally shouldn't be exposed directly.
sdk/search/Azure.Search.Documents/api/Azure.Search.Documents.netstandard2.0.cs
Outdated
Show resolved
Hide resolved
sdk/search/Azure.Search.Documents/api/Azure.Search.Documents.netstandard2.0.cs
Outdated
Show resolved
Hide resolved
| public partial class CustomEntity | ||
| { | ||
| /// <summary> An array of complex objects that can be used to specify alternative spellings or synonyms to the root entity name. </summary> | ||
| public IList<CustomEntityAlias> Aliases { get; } |
There was a problem hiding this comment.
This was necessary to prevent the setters? @pakrym I'm concerned this is a regression. These weren't settable before. The last thread I can find on the matter, we were going to try an AddRange extension method first.
That said, this shouldn't hold up this PR. You're doing the right thing here, @Mohit-Chakraborty, it just shouldn't be necessary as, without it, it goes against .NET guidelines we haven't formerly overridden.
There was a problem hiding this comment.
Yes, this is to remove setters from collection properties.
| public partial class DocumentExtractionSkill | ||
| { | ||
| /// <summary> A dictionary of configurations for the skill. </summary> | ||
| public IDictionary<string, object> Configuration { get; } |
There was a problem hiding this comment.
Is this a dictionary of or containing well-known key-value pairs? See the IndexingParameterConfiguration class for what we did in that case. Note that if this is the first time something like this is shipping, avoid all the casting magic I do and just declare it as an expandable property bag with well-known properties the first time. We only shipped the SDK that way previously because it had to GA without the well-known autorest configuration finalized and we couldn't break the API later.
sdk/search/Azure.Search.Documents/src/Indexes/Models/SearchServiceCounters.cs
Show resolved
Hide resolved
sdk/search/Azure.Search.Documents/src/Models/SearchQueryType.Serialization.cs
Show resolved
Hide resolved
sdk/search/Azure.Search.Documents/src/Models/SearchQueryType.Serialization.cs
Outdated
Show resolved
Hide resolved
tg-msft
left a comment
tg-msft
left a comment
There was a problem hiding this comment.
Looks great overall - left a couple of small comments.
sdk/search/Azure.Search.Documents/src/Indexes/Models/SearchIndex.cs
Outdated
Show resolved
Hide resolved
| public DocumentExtractionSkill(System.Collections.Generic.IEnumerable<Azure.Search.Documents.Indexes.Models.InputFieldMappingEntry> inputs, System.Collections.Generic.IEnumerable<Azure.Search.Documents.Indexes.Models.OutputFieldMappingEntry> outputs) { } | ||
| public System.Collections.Generic.IDictionary<string, object> Configuration { get { throw null; } } | ||
| public string DataToExtract { get { throw null; } set { } } | ||
| public string ParsingMode { get { throw null; } set { } } |
There was a problem hiding this comment.
I think this should be an enum which means we probably have a swagger bug? Fine for this preview, but we should dig into it.
AnswerResult,Answers,CaptionResult,QueryLanguage,SpellerCharFilterNameCustomEntityLookupSkill,DocumentExtractionSkillLexicalNormalizeras a search fieldRoundtripAllSkillstest and add session recordCustomEntity,CustomEntityLookupSkillandDocumentExtractionSkill