Azure-sdk-for-net: Search: Proposed model and property renames

[heaths]

This is a list of proposed changes for models and properties. Some of these changes may have already been done in one or more languages, but I wanted to start an issue to track all of them and, when agreed, will replicate across the different language repos.

Model | Property | Proposal | Notes
--- | --- | --- | ---
Analyzer | | LexicalAnalyzer
AnalyzerName | | LexicalAnalyzerName
AzureActiveDirectoryApplicationCredentials | | remove | Just uplift ApplicationId and ApplicationSecret to SearchIndexEncryptionKey
DataContainer | | SearchIndexerDataContainer
DataSource | | SearchIndexerDataSource
DataSource | Credentials | ConnectionString (string) | See notes below for DataSourceCredentials
DataSourceCredentials | | remove | Just uplift ConnectionString to SearchDataSource
DataSourceType | | SearchIndexerDataSourceType
DataType | | SearchFieldDataType
EncryptionKey | | SearchResourceEncryptionKey
EncryptionKey | KeyVaultUri | VaultUri | Corresponds with Key Vault naming.
EncryptionKey | KeyVaultName | KeyName | Corresponds with Key Vault naming.
EncryptionKey | KeyVaultVersion | KeyVersion | Corresponds with Key Vault naming.
Field | | SearchField
FieldBase | | SearchFieldTemplate | Probably specific to .NET - base class for convenience classes.
Index | | SearchIndex
Indexer | | SearchIndexer
IndexerExecutionInfo | | SearchIndexerStatus | Corresponds to SearchServiceClient.GetIndexerStatus
IndexerLimits | | SearchIndexerLimits
ItemError | | SearchIndexerError
ItemWarning | | SearchIndexerWarning
LengthTokenFilter | Max | LengthTokenFilter.MaxLength
LengthTokenFilter | Min | LengthTokenFilter.MinLength
Skill | | SearchIndexerSkill
Skillset | | SearchIndexerSkillset
StandardAnalyzer | | LuceneStandardAnalyzer
StandardTokenizer | | LuceneStandardTokenizer
TextInfo | | AnalyzedTokenInfo
Tokenizer | | LexicalTokenizer
TokenizerName | | LexicalTokenizerName

On a related note, other models should be hidden and their sole collection properties uplifted as return values. Names below reflect proposed renames above.

Method | Model | Return
--- | --- | ---
SearchServiceClient.ListDataSources | ListDataSourcesResult | DataSource[]
SearchServiceClient.ListIndexers | ListIndexersResult | SearchIndexer[]
SearchServiceClient.ListIndexes | ListIndexesResult | SearchIndex[]
SearchServiceClient.ListSkillsets | ListSkillsetsResult | SearchSkillset[]
SearchServiceClient.ListSynonymMaps | ListSynonymMapsResult | SynonymMap[]

[heaths]

Waiting on @brjohnstmsft and team for a counter proposal to some of the naming proposals.

[brjohnstmsft]

Here are my counter-proposals for some of the names. A lot of these are born of the fact that prefixing everything with "Search" will often confuse more than it will help. This is because Search, like Index, is sometimes a noun and sometimes a verb.

Model | Property | Proposal | Counter-proposal
--- | --- | --- | ---
Analyzer | | SearchAnalyzer | LexicalAnalyzer
AnalyzerName | | SearchAnalyzerName | LexicalAnalyzerName
DataType | | SearchFieldDataType | I don't love it because it's too long, but it seems better than SearchDataType, so OK
Field | | SearchIndexField | SearchField
FieldBase | | SearchField | SearchFieldTemplate
Index | | SearchIndex | OK
LengthTokenFilter | Max | LengthTokenFilter.MaxLength | OK
LengthTokenFilter | Min | LengthTokenFilter.MinLength | OK
StandardAnalyzer | | StandardSearchAnalyzer | LuceneStandardAnalyzer
StandardTokenizer | | StandardSearchTokenizer | LuceneStandardTokenizer
TextInfo | | TextAnalysisInfo | AnalyzedTokenInfo (there is no TextInfo type)
Tokenizer | | SearchTokenizer | LexicalTokenizer
TokenizerName | | SearchTokenizerName | LexicalTokenizerName

I'm ok with all of these:

Method | Model | Return
--- | --- | ---
SearchServiceClient.ListDataSources | ListDataSourcesResult | DataSource[]
SearchServiceClient.ListIndexers | ListIndexersResult | SearchIndexer[]
SearchServiceClient.ListIndexes | ListIndexesResult | SearchIndex[]
SearchServiceClient.ListSkillsets | ListSkillsetsResult | SearchSkillset[]
SearchServiceClient.ListSynonymMaps | ListSynonymMapsResult | SynonymMap[]

Waiting for @shmed to weigh in on encryption key properties, and @bleroy to relay the counter-proposals from his team for indexer, datasource, and skill-related names.

[bleroy]

And here's our counter-proposal for the indexer/enrichments part:

Ideally, we'd prefer to capture context in the namespace rather than prefixing type names. With that option, we'd have a Azure.Search.Indexers namespace with the classes Indexer, Skillset, Skill and DataSource.

If that's not the option that's selected, our second choice is to prefix with SearchIndexer to reflect the rename of the Indexer class itself. That gives the following type names:

• SearchIndexer
• SearchIndexerSkillset
• SearchIndexerSkill
• SearchIndexerDataSource
• SearchIndexerError
• SearchIndexerWarning

[heaths]

Some languages can't have parent "namespaces" in the same package, so we are left with class prefixes as a fall back. Do either of you or anyone else on your team have any more counter proposals for classes here? If not, I'll update the table here and in the other languages' repos where I opened copies.

[heaths]

@brjohnstmsft @bleroy could you have a look over the updated issue description and make sure I captured everything correctly? I did make one change. With SearchIndexerDataSource, I also similarly renamed SearchIndexerDataSourceContainer. It's long, but groupable.

[bleroy]

LGTM

[brjohnstmsft]

@bleroy Just to clarify, should it be SearchIndexerDataSourceContainer, or just SearchIndexerDataContainer?

[brjohnstmsft]

@heaths I think @shmed mentioned this elsewhere -- SearchIndexEncryptionKey is misleading because these keys can be used to secure other resources besides indexes (synonym maps for now, possibly others to follow). I would propose either SearchEncryptionKey or maybe SearchResourceEncryptionKey.

[shmed]

Yes, thank you @brjohnstmsft from bringing this to attention in this thread. SearchEncryptionKey or SearchResourceEncryptionKey both sounds good to me. Do we use the term "Resource" elsewhere in the API? Also, as you mentioned, @arv100kri is bringing encryption keys to skillsets, indexers and data sources, so we should definitely keep the name generic.

[heaths]

I think either is fine. SearchResourceEncryptionKey is more descriptive, albeit long. Sorry I missed that.

As for "container", it's only used in a DataSource but I'd be fine with leaving it just SearchIndexerDataContainer. It fits with the original name. I'll make the change.

[bleroy]

@brjohnstmsft SearchIndexerDataContainer