Board Review: Azure Cognitive Search SDK for Java - preview 2

[CodeRunRepeat]

The Basics

• Service team responsible for the client library: Azure Cognitive Search (Bruce Johnston @brjohnstmsft brjohnst@microsoft.com) implemented by Commercial Software Engineering
• Link to documentation describing the service: https://docs.microsoft.com/en-us/azure/search/
• Contact email (if service team, provide PM and Dev Lead):
  • CSE Dev Lead: @navalev navale@microsoft.com
  • CSE Dev Crew Lead: @tamirkamara takamara@microsoft.com
  • CSE TPM: @CodeRunRepeat anastm@microsoft.com

About this client library

• Name of the client library: Azure Cognitive Search SDK for Java - preview 2
• Languages for this review: Java
• Link to the service REST APIs: https://docs.microsoft.com/en-us/rest/api/searchservice/

Artifacts required (per language)

• API view link: https://apiview.azurewebsites.net/Assemblies/Review/2abf935de65e4187964bea5b817872d6

• Link to samples for champion scenarios: TBD - under development in https://github.com/navalev/azure-sdk-for-java-pr/

Champion Scenarios

Scenario 1: Setting up a basic search solution

This scenario assumes an existing search service

• Create a data source for a Cosmos DB database
• Create an index with a small set of fields
• Create a skillset for Cognitive Services
• Create an indexer that loads the index from the data source using the skillset
• Set up an indexer run schedule

Scenario 2: Running a search solution

This scenario assumes an existing search solution as set up in scenario 1

• Get index statistics
• Run the indexer on demand
• Perform a search
• Run an autocomplete and suggest query

Scenario 3: Refining search capabilities

This scenario assumes an existing search solution as set up in scenario 1

• Add a synonym map to an index field
• Add a language detection skill, a conditional skill that replaces nulls with "N/A", and a custom web api skill to the indexer
• Manually add a set of documents to the index
• Retrieve service statistics

Agenda for the review

A board review is generally split into two parts, with additional meetings as required

Part 1 - Introducing the board to the service:

• Review of the service (no more than 10 minutes).
• Review of the champion scenarios.
• Get feedback on the API patterns used in the champion scenarios.

After part 1, you may schedule additional meetings with architects to refine the API and work on implementation.

Part 2 - the "GA" meeting

• Scheduled at least one week after the APIs have been uploaded for review.
• Will go over controversial feedback from the line-by-line API review.
• Exit meeting with concrete changes necessary to meet quality bar.

Thank you for your submission

[CodeRunRepeat]

@JonathanGiles @brjohnstmsft we have now closed all the bugs that were captured as part of this review. The new revision of the API is available at https://apiview.azurewebsites.net/Assemblies/Revisions/2abf935de65e4187964bea5b817872d6.

Here is the full list of related work items. Please let us know what else is needed to close the review.
cc @adrianhall

Work Item	Title	Tags	Reason	PRs
Bug 1626	Consider reshaping Analyzer and derived classes	#adp; #autorest	Deferred
Bug 1631	Review ConditionalSkill class	#adp; #autorest_config	As Designed
Bug 1698	DataSources static methods naming	#adp; blocked	Fixed and verified
Bug 1615	Review "autocomplete" naming vs. "autoComplete"	#adp; #swagger	As Designed
Bug 1627	Review naming for AutocompleteOptions.getTop()/setTop()	#adp; #autorest_config	As Designed
Bug 1612	Review use case and base type for Document()	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/306
Bug 1619	Review overloads in service plane methods wrt "withResponse" (async client)	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/314
Bug 1620	Use datasource or dataSource consistently throughout methods and parameters	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/307
Bug 1632	Consider renaming IndexGetStatisticsResult + model classes	#adp; #swagger	Deferred
Bug 1678	Add createDataSource methods to SearchService(Async)Client	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/326
Bug 1613	JavaBeans naming for RangeFacetResult properties	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1616	Review the need for "withResponse" "overloads" in autocomplete	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/314
Bug 1621	JavaBeans naming for ValueFacetResult properties	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1623	JavaBeans naming for SearchPagedResponse properties	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1624	JavaBeans naming for SuggestPagedResponse properties	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/316
Bug 1629	Research overlap between generated AutocompleteOptions and AutocompleteRequest classes	#adp; #autorest_config	As Designed
Bug 1622	Review static DataSources methods that create a new DataSource	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/330
Bug 1625	Consider hiding AnalyzeResult and AutocompleteResult	#adp; #autorest_config	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/338
Bug 1630	Rename CognitiveServicesByKey	#adp; #swagger	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/338
				Azure/azure-rest-api-specs#7859
Bug 1618	Implement required settings in SearchIndexClientBuilder	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/334
Bug 1614	In Search()()Client classes, change getApiVersion()	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/334
Bug 1617	Consider renaming ApiKeyCredentials	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/329
Bug 1696	Change references to Cosmos DB to Cosmos	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/331
Bug 1697	Ensure there aren't any public classes that are not part of the public API	#adp	Verified	https://github.com/navalev/azure-sdk-for-java-pr/pull/338

ADP2 report.xlsx

[brjohnstmsft]

Looks good to me, except for one item: Bug 1632 | Consider renaming IndexGetStatisticsResult + model classes

I've made the Swagger change for that in this PR. Please review and re-generate the Java SDK code accordingly.

[CodeRunRepeat]

Thanks @brjohnstmsft will do as soon as our repo is resurrected.

[navalev]

@brjohnstmsft code has been regenerated following the swagger spec changes as per https://github.com/navalev/azure-sdk-for-java-pr/pull/349

[CodeRunRepeat]

@brjohnstmsft @JonathanGiles can we close this?
cc @navalev

[brjohnstmsft]

I'm fine closing it if @JonathanGiles is.