-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
code refactoring 4 #11800
code refactoring 4 #11800
Changes from 7 commits
0e8db7f
2cb3750
da6d4f5
902db93
119e4e8
bf94e32
88f8895
25a0755
d4cf252
6212964
af02a00
0c3f697
6376587
a3f66a8
42e60ec
13d56f4
a544404
a6d0fa7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,12 +5,10 @@ | |
# -------------------------------------------------------------------------- | ||
from typing import cast, List, TYPE_CHECKING | ||
|
||
import six | ||
|
||
from azure.core.tracing.decorator_async import distributed_trace_async | ||
from ._paging import AsyncSearchItemPaged, AsyncSearchPageIterator | ||
from .._generated.aio import SearchIndexClient | ||
from .._generated.models import IndexBatch, IndexingResult, SearchRequest | ||
from .._generated.models import IndexBatch, IndexingResult | ||
from .._index_documents_batch import IndexDocumentsBatch | ||
from .._queries import AutocompleteQuery, SearchQuery, SuggestQuery | ||
from ..._headers_mixin import HeadersMixin | ||
|
@@ -104,12 +102,12 @@ async def get_document(self, key, selected_fields=None, **kwargs): | |
return cast(dict, result) | ||
|
||
@distributed_trace_async | ||
async def search(self, query, **kwargs): | ||
# type: (Union[str, SearchQuery], **Any) -> AsyncSearchItemPaged[dict] | ||
async def search(self, search_text, **kwargs): | ||
# type: (str, **Any) -> AsyncSearchItemPaged[dict] | ||
"""Search the Azure search index for documents. | ||
|
||
:param query: An query for searching the index | ||
:type documents: str or SearchQuery | ||
:param str search_text: A full-text search query expression; Use "*" or omit this parameter to | ||
match all documents. | ||
:rtype: AsyncSearchItemPaged[dict] | ||
|
||
.. admonition:: Example: | ||
|
@@ -139,22 +137,50 @@ async def search(self, query, **kwargs): | |
:dedent: 4 | ||
:caption: Get search result facets. | ||
""" | ||
if isinstance(query, six.string_types): | ||
query = SearchQuery(search_text=query) | ||
elif not isinstance(query, SearchQuery): | ||
raise TypeError( | ||
"Expected a string or SearchQuery for 'query', but got {}".format( | ||
repr(query) | ||
) | ||
) | ||
include_total_result_count = kwargs.pop("include_total_result_count", None) | ||
facets = kwargs.pop("facets", None) | ||
filter_arg = kwargs.pop("filter", None) | ||
highlight_fields = kwargs.pop("highlight_fields", None) | ||
highlight_post_tag = kwargs.pop("highlight_post_tag", None) | ||
highlight_pre_tag = kwargs.pop("highlight_pre_tag", None) | ||
minimum_coverage = kwargs.pop("minimum_coverage", None) | ||
order_by = kwargs.pop("order_by", None) | ||
query_type = kwargs.pop("query_type", None) | ||
scoring_parameters = kwargs.pop("scoring_parameters", None) | ||
scoring_profile = kwargs.pop("scoring_profile", None) | ||
search_fields = kwargs.pop("search_fields", None) | ||
search_mode = kwargs.pop("search_mode", None) | ||
select = kwargs.pop("select", None) | ||
skip = kwargs.pop("skip", None) | ||
top = kwargs.pop("top", None) | ||
query = SearchQuery( | ||
search_text=search_text, | ||
include_total_result_count=include_total_result_count, | ||
facets=facets, | ||
filter=filter_arg, | ||
highlight_fields=highlight_fields, | ||
highlight_post_tag=highlight_post_tag, | ||
highlight_pre_tag=highlight_pre_tag, | ||
minimum_coverage=minimum_coverage, | ||
order_by=order_by, | ||
query_type=query_type, | ||
scoring_parameters=scoring_parameters, | ||
scoring_profile=scoring_profile, | ||
search_fields=search_fields, | ||
search_mode=search_mode, | ||
select=select, | ||
skip=skip, | ||
top=top | ||
) | ||
Comment on lines
+140
to
+174
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit - Is it worth moving this logic onto the SearchQuery class so you don't have to update both the sync/async clients whenever we add a new parameter? I'm not sure how the Python folks typically handle these situations. (Also, we can just file a work item rather for Preview 5 rather than try to fix it today.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SearchQuery and AutocompleteQuery, SuggestQuery share one ctor. If we put all of pop operation there, I am afraid it will be messed up. |
||
|
||
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers")) | ||
return AsyncSearchItemPaged( | ||
self._client, query, kwargs, page_iterator_class=AsyncSearchPageIterator | ||
) | ||
|
||
@distributed_trace_async | ||
async def suggest(self, search_text, suggester_name, **kwargs): | ||
# type: (Union[str, SuggestQuery], **Any) -> List[dict] | ||
# type: (str, str, **Any) -> List[dict] | ||
"""Get search suggestion results from the Azure search index. | ||
|
||
:param str search_text: Required. The search text to use to suggest documents. Must be at least 1 | ||
|
@@ -172,7 +198,28 @@ async def suggest(self, search_text, suggester_name, **kwargs): | |
:dedent: 4 | ||
:caption: Get search suggestions. | ||
""" | ||
query = SuggestQuery(search_text=search_text, suggester_name=suggester_name, **kwargs) | ||
filter_arg = kwargs.pop("filter", None) | ||
use_fuzzy_matching = kwargs.pop("use_fuzzy_matching", None) | ||
highlight_post_tag = kwargs.pop("highlight_post_tag", None) | ||
highlight_pre_tag = kwargs.pop("highlight_pre_tag", None) | ||
minimum_coverage = kwargs.pop("minimum_coverage", None) | ||
order_by = kwargs.pop("order_by", None) | ||
search_fields = kwargs.pop("search_fields", None) | ||
select = kwargs.pop("select", None) | ||
top = kwargs.pop("top", None) | ||
query = SuggestQuery( | ||
search_text=search_text, | ||
suggester_name=suggester_name, | ||
filter=filter_arg, | ||
use_fuzzy_matching=use_fuzzy_matching, | ||
highlight_post_tag=highlight_post_tag, | ||
highlight_pre_tag=highlight_pre_tag, | ||
minimum_coverage=minimum_coverage, | ||
order_by=order_by, | ||
search_fields=search_fields, | ||
select=select, | ||
top=top | ||
) | ||
|
||
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers")) | ||
response = await self._client.documents.suggest_post( | ||
|
@@ -200,7 +247,26 @@ async def autocomplete(self, search_text, suggester_name, **kwargs): | |
:dedent: 4 | ||
:caption: Get a auto-completions. | ||
""" | ||
query = AutocompleteQuery(search_text=search_text, suggester_name=suggester_name, **kwargs) | ||
autocomplete_mode = kwargs.pop("autocomplete_mode", None) | ||
filter_arg = kwargs.pop("filter", None) | ||
use_fuzzy_matching = kwargs.pop("use_fuzzy_matching", None) | ||
highlight_post_tag = kwargs.pop("highlight_post_tag", None) | ||
highlight_pre_tag = kwargs.pop("highlight_pre_tag", None) | ||
minimum_coverage = kwargs.pop("minimum_coverage", None) | ||
search_fields = kwargs.pop("search_fields", None) | ||
top = kwargs.pop("top", None) | ||
query = AutocompleteQuery( | ||
search_text=search_text, | ||
suggester_name=suggester_name, | ||
autocomplete_mode=autocomplete_mode, | ||
filter=filter_arg, | ||
use_fuzzy_matching=use_fuzzy_matching, | ||
highlight_post_tag=highlight_post_tag, | ||
highlight_pre_tag=highlight_pre_tag, | ||
minimum_coverage=minimum_coverage, | ||
search_fields=search_fields, | ||
top=top | ||
) | ||
|
||
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers")) | ||
response = await self._client.documents.autocomplete_post( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -235,7 +235,7 @@ def create_or_update_index( | |
|
||
@distributed_trace | ||
def analyze_text(self, index_name, analyze_request, **kwargs): | ||
# type: (str, AnalyzeRequest, **Any) -> AnalyzeResult | ||
# type: (str, AnalyzeTextRequest, **Any) -> AnalyzeResult | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already fixed. :) |
||
"""Shows how an analyzer breaks text into tokens. | ||
|
||
:param index_name: The name of the index for which to test an analyzer. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,14 +32,11 @@ async def filter_query(): | |
# [START facet_query_async] | ||
from azure.core.credentials import AzureKeyCredential | ||
from azure.search.documents.aio import SearchClient | ||
from azure.search.documents.models import SearchQuery | ||
|
||
search_client = SearchClient(service_endpoint, index_name, AzureKeyCredential(key)) | ||
|
||
query = SearchQuery(search_text="WiFi", facets=["Category"], top=0) | ||
|
||
async with search_client: | ||
results = await search_client.search(query=query) | ||
results = await search_client.search(search_text="WiFi", facets=["Category"], top=0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks very nice. |
||
|
||
facets = await results.get_facets() | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by omit this parameter - do you mean search_text is optional? If not, is it by design to not make it optional and use '*' as default value.