update version number and API_version support by xiangyan99 · Pull Request #12154 · Azure/azure-sdk-for-python

xiangyan99 · 2020-06-22T23:38:02Z

No description provided.

xiangyan99 · 2020-06-23T19:46:49Z

/azp run python - search - ci

azure-pipelines · 2020-06-23T19:47:03Z

Azure Pipelines successfully started running 1 pipeline(s).

xiangyan99 · 2020-06-23T23:25:39Z

#12171

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py

heaths

Approving to keep you unblocked from making fixes and submitting, but please see comments. I also see now this was to change the package version and not the API version, but please note that is also necessary.

sdk/search/azure-search-documents/CHANGELOG.md

heaths · 2020-06-25T17:45:45Z

sdk/search/azure-search-documents/azure/search/documents/_api_versions.py

+# ------------------------------------
+
+_SUPPORTED_API_VERSIONS = [
+    "2019-05-06-Preview",


If I understand the PR correctly, it's to make sure this is "2020-06-03" everywhere.

We will have a separate PR to update the API version number (that will include updating auto-generated code)

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py

heaths · 2020-06-25T17:56:06Z

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py


        :param str search_text: A full-text search query expression; Use "*" or omit this parameter to
        match all documents.
+        :keyword include_total_result_count: A value that specifies whether to fetch the total count of


@tg-msft, shouldn't this be include_total_count? Or was that one of the things we were asked to change? I don't see it changed in .NET yet.

include_total_result_count is from the swagger. Do we want to change it?

heaths · 2020-06-25T18:13:07Z

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py

        :param str search_text: The search text on which to base autocomplete results.
        :param str suggester_name: The name of the suggester as specified in the suggesters
        collection that's part of the index definition.
+        :keyword autocomplete_mode: Specifies the mode for Autocomplete. The default is 'oneTerm'. Use


In .NET we just have this as mode. @tg-msft, should it match in this case or is this idiomatically acceptable?

/cc @johanste

I am ok to use mode, just want to confirm because autocomplete_mode is from swagger.

sdk/search/azure-search-documents/README.md

lmazuel · 2020-06-25T18:22:49Z

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py

    def __init__(self, endpoint, index_name, credential, **kwargs):
        # type: (str, str, AzureKeyCredential, **Any) -> None

+        get_api_version(kwargs, "2019-05-06-Preview")


So I guess here it's used has a "check", then I would split a "check" and a "get" into two different methods, with "get" calling check first. I find it easier to read intent

It's optional feedback, I let you decide if that makes the code less efficient

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py

lmazuel · 2020-06-25T20:34:39Z

sdk/search/azure-search-documents/azure/search/documents/_internal/aio/_search_client_async.py


+        api_version = kwargs.pop('api_version', None)
+        if api_version:
+            check_api_version(api_version)


I see a lot the pattern "if api_version: check", does it mean check should not raise if api_version is None? And then simplify all those occurences?

lmazuel

Approving the Python side of things, no feedback on Search semantic

rakshith91 · 2020-06-25T21:18:58Z

sdk/search/azure-search-documents/azure/search/documents/_api_versions.py

+
+
+def get_api_version(kwargs, default):
+    # type: (Dict[str, Any]) -> str


is the type hint correct? there are 2 params above.

Good catch! Fixed.

…into regenerate_certs * 'master' of https://github.com/Azure/azure-sdk-for-python: (27 commits) Set http_logging_policy in Configuration (Azure#12218) Sync eng/common directory with azure-sdk-tools repository (Azure#11990) AzureCliCredential instructs CLI not to color output (Azure#11362) Sdk automation/track2 azure mgmt storage (Azure#12238) Fix changelog of CustomVision (Azure#12225) Doc update for conf file name (Azure#12224) update doc for content_type (Azure#12220) update API version to use 2020-06-30 (Azure#12208) Use MSAL's custom transport API (Azure#11892) add breadcumbs for training filter (Azure#12196) [formrecognizer] arch board feedback renames (Azure#12207) Dataplane autogeneration (Azure#12210) update version number and API_version support (Azure#12154) Update SECURITY.md (Azure#12209) adding a pip freeze to ensure we fully understand what our environment has (Azure#12173) FaceAPI 0.4.1 (Azure#12199) [ServiceBus] Enable Async Unittest (Azure#12001) add search owver (Azure#12190) update training docstring (Azure#12168) don't use mgmt track2 (Azure#12183) ...

…into regenerate_keys * 'master' of https://github.com/Azure/azure-sdk-for-python: Set http_logging_policy in Configuration (Azure#12218) Sync eng/common directory with azure-sdk-tools repository (Azure#11990) AzureCliCredential instructs CLI not to color output (Azure#11362) Sdk automation/track2 azure mgmt storage (Azure#12238) Fix changelog of CustomVision (Azure#12225) Doc update for conf file name (Azure#12224) update doc for content_type (Azure#12220) update API version to use 2020-06-30 (Azure#12208) Use MSAL's custom transport API (Azure#11892) add breadcumbs for training filter (Azure#12196) [formrecognizer] arch board feedback renames (Azure#12207) Dataplane autogeneration (Azure#12210) update version number and API_version support (Azure#12154) Update SECURITY.md (Azure#12209) adding a pip freeze to ensure we fully understand what our environment has (Azure#12173) FaceAPI 0.4.1 (Azure#12199) [ServiceBus] Enable Async Unittest (Azure#12001) add search owver (Azure#12190) update training docstring (Azure#12168)

…into regenerate_secrets * 'master' of https://github.com/Azure/azure-sdk-for-python: (36 commits) Install dev dependency when running apistub (Azure#12268) SharedTokenCacheCredential lazily loads the cache (Azure#12172) Changes in docs [Form Recognizer] (Azure#12216) [formrecognizer] adjust text angle to fit in specified interval (Azure#12248) Set http_logging_policy in Configuration (Azure#12218) Sync eng/common directory with azure-sdk-tools repository (Azure#11990) AzureCliCredential instructs CLI not to color output (Azure#11362) Sdk automation/track2 azure mgmt storage (Azure#12238) Fix changelog of CustomVision (Azure#12225) Doc update for conf file name (Azure#12224) update doc for content_type (Azure#12220) update API version to use 2020-06-30 (Azure#12208) Use MSAL's custom transport API (Azure#11892) add breadcumbs for training filter (Azure#12196) [formrecognizer] arch board feedback renames (Azure#12207) Dataplane autogeneration (Azure#12210) update version number and API_version support (Azure#12154) Update SECURITY.md (Azure#12209) adding a pip freeze to ensure we fully understand what our environment has (Azure#12173) FaceAPI 0.4.1 (Azure#12199) ...

update

a746d2c

xiangyan99 requested review from heaths and rakshith91 June 22, 2020 23:38

xiangyan99 requested review from lmazuel and mayurid as code owners June 22, 2020 23:38

update version

d3b9a21

rakshith91 reviewed Jun 25, 2020

View reviewed changes

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py Outdated Show resolved Hide resolved

xiangyan99 changed the title ~~update~~ update version number and API_version support Jun 25, 2020

heaths previously approved these changes Jun 25, 2020

View reviewed changes

lmazuel reviewed Jun 25, 2020

View reviewed changes

sdk/search/azure-search-documents/README.md Outdated Show resolved Hide resolved

lmazuel reviewed Jun 25, 2020

View reviewed changes

sdk/search/azure-search-documents/README.md Outdated Show resolved Hide resolved

lmazuel reviewed Jun 25, 2020

View reviewed changes

sdk/search/azure-search-documents/README.md Show resolved Hide resolved

lmazuel reviewed Jun 25, 2020

View reviewed changes

sdk/search/azure-search-documents/azure/search/documents/_internal/_search_client.py Outdated Show resolved Hide resolved

update

61b036c

xiangyan99 dismissed heaths’s stale review via 61b036c June 25, 2020 18:56

xiangyan99 added 2 commits June 25, 2020 12:00

remove default api version in docstring

fc5212d

update kwarg names

123f2d6

lmazuel reviewed Jun 25, 2020

View reviewed changes

lmazuel previously approved these changes Jun 25, 2020

View reviewed changes

update

b64ccac

xiangyan99 dismissed lmazuel’s stale review via b64ccac June 25, 2020 20:46

rakshith91 reviewed Jun 25, 2020

View reviewed changes

update

13baf4e

rakshith91 approved these changes Jun 25, 2020

View reviewed changes

xiangyan99 merged commit ace79c0 into master Jun 25, 2020

xiangyan99 deleted the search_review_feedback branch June 25, 2020 21:56

xiangyan99 mentioned this pull request Jun 29, 2020

Change Azure.Search.Documents version to 11.0.0 #12171

Closed



		def get_api_version(kwargs, default):
		# type: (Dict[str, Any]) -> str

Conversation

xiangyan99 commented Jun 22, 2020

Uh oh!

xiangyan99 commented Jun 23, 2020

Uh oh!

azure-pipelines bot commented Jun 23, 2020

Uh oh!

xiangyan99 commented Jun 23, 2020

Uh oh!

Uh oh!

heaths left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiangyan99 Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmazuel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xiangyan99 Jun 25, 2020 •

edited

Loading