Search Renames / Regen #11342
Search Renames / Regen #11342
Conversation
| @@ -43,9 +43,9 @@ | |||
| edm, | |||
| ) | |||
| from ._service._generated.models import ( | |||
| Analyzer, | |||
| AnalyzeRequest, | |||
There was a problem hiding this comment.
Hard to tell from the diff, but these *Request and *Result classes aren't actually exported, are they? They should most often (always, IIRC) be wrapped/transformed before returning.
There was a problem hiding this comment.
Same for DataSourceCredentials I see below.
There was a problem hiding this comment.
Yes they have been used directly since awhile ago: https://github.com/Azure/azure-sdk-for-python/blob/master/sdk/search/azure-search-documents/samples/sample_analyze_text.py
| LimitTokenFilter, | ||
| LuceneStandardAnalyzer, | ||
| LuceneStandardTokenizer, |
There was a problem hiding this comment.
There should also be a (new) LuceneStandardTokenizerV2 that was added a couple weeks ago.
...arch-documents/azure/search/documents/_index/_generated/models/_search_index_client_enums.py
Show resolved
Hide resolved
...earch-documents/azure/search/documents/_index/_generated/operations/_documents_operations.py
Show resolved
Hide resolved
| @@ -15,7 +15,7 @@ | |||
|
|
|||
| if TYPE_CHECKING: | |||
| # pylint:disable=unused-import,ungrouped-imports | |||
| from ._generated.models import DataSource | |||
| from ._generated.models import SearchIndexerDataSource | |||
There was a problem hiding this comment.
The client name should be {Model}Client here and elsewhere, so SearchIndexerDataSourceClient. Getters from SearchServiceClient should match (for now; we're discussing with archboard but this would be currently consistent).
There was a problem hiding this comment.
This is the name of the model in the generated code, not the convenience layer client
There was a problem hiding this comment.
The current name of this client is SearchDataSourcesClient so I guess it would also need to be updated (in a different place)
| 'replacement': {'key': 'replacement', 'type': 'str'}, | ||
| 'description': {'key': 'description', 'type': 'str'}, | ||
| 'type': {'key': 'type', 'type': 'str'}, | ||
| 'credentials': {'key': 'credentials', 'type': 'DataSourceCredentials'}, |
There was a problem hiding this comment.
The only propery on this - ConnectionString - should actually be on the SearchIndexerDataSource type. This isn't passed in by the user, is it?
There was a problem hiding this comment.
They currently do. This will be a change to the current samples too.
|
Well, that's what I get for trying to resolve a merge conflict from the web UI |
|
I think I'd be okay with that, but I'm not really sure how much of the renames this PR that says it's about "renames" contains. If you're taking it in pieces, fine, but maybe get a version uploaded to APIView (just the text output of APIs, right?) ASAP so we can compare. It's hard enough in GitHub to see the renames, but harder still when spread across multiple PRs. APIView or even just a gist of before/after would probably be easier to see if anything was missed. |
@heaths I made an attempt at atomic commits just so that this would be transparent: If it helps I could add some indications/markings in the table in #10860 |
|
Annotations in that bug could help (or just remove the ones you did already - it's all tracked); though, even if we just diff the API somehow when you're done, that works as well. Whatever is easier for both you and reviewers like me. I'm not against piecemeal changes, just saying it's harder to see how much was done. 🙂 |
…into make_canary_test * 'master' of https://github.com/Azure/azure-sdk-for-python: (28 commits) Kaihuis maps (Azure#11574) [formrecognizer] support Copy API (Azure#11372) Artifact Powered Docs.MS Release (Azure#11395) Sync eng/common directory with azure-sdk-tools repository (Azure#11562) Updated changelog (Azure#11554) update pinned versions in autorest_req.txt (Azure#11557) Add user authentication API to UsernamePasswordCredential (Azure#11528) Reduce Build Matrix (Azure#11539) Prevent Key Vault test failure due to operation timing (Azure#11552) update tests (Azure#11534) Datashare 2019 11 01 (Azure#11540) Search Renames / Regen (Azure#11342) update unit test according to the latest uamqp update (Azure#11533) Sync eng/common directory with azure-sdk-tools repository (Azure#11472) rename input parameters (Azure#11518) [cosmos] readme review feedback (Azure#11527) Update CODEOWNERS (Azure#11516) [Core] Support multipart changesets (Azure#10972) fix AttributeException (Azure#11463) release-for-hanaonazure-mgmt (Azure#11441) ...
…into feature/text_analytics_v3.0 * 'master' of https://github.com/Azure/azure-sdk-for-python: (28 commits) Kaihuis maps (#11574) [formrecognizer] support Copy API (#11372) Artifact Powered Docs.MS Release (#11395) Sync eng/common directory with azure-sdk-tools repository (#11562) Updated changelog (#11554) update pinned versions in autorest_req.txt (#11557) Add user authentication API to UsernamePasswordCredential (#11528) Reduce Build Matrix (#11539) Prevent Key Vault test failure due to operation timing (#11552) update tests (#11534) Datashare 2019 11 01 (#11540) Search Renames / Regen (#11342) update unit test according to the latest uamqp update (#11533) Sync eng/common directory with azure-sdk-tools repository (#11472) rename input parameters (#11518) [cosmos] readme review feedback (#11527) Update CODEOWNERS (#11516) [Core] Support multipart changesets (#10972) fix AttributeException (#11463) release-for-hanaonazure-mgmt (#11441) ...
No description provided.