Skip to content
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

Define match conditions for CreateOrUpdate and Delete operations #11116

Merged
merged 18 commits into from
May 1, 2020
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
StopAnalyzer,
StopwordsTokenFilter,
Suggester,
SynonymMap,
SynonymTokenFilter,
TagScoringFunction,
TagScoringParameters,
Expand Down Expand Up @@ -203,6 +204,7 @@
"StopwordsTokenFilter",
"SuggestQuery",
"Suggester",
"SynonymMap",
"SynonymTokenFilter",
"TagScoringFunction",
"TagScoringParameters",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@
# --------------------------------------------------------------------------
from typing import TYPE_CHECKING

from azure.core import MatchConditions
from azure.core.tracing.decorator import distributed_trace

from ._generated import SearchServiceClient as _SearchServiceClient
from ._utils import get_access_conditions
from .._headers_mixin import HeadersMixin
from .._version import SDK_MONIKER

if TYPE_CHECKING:
# pylint:disable=unused-import,ungrouped-imports
from ._generated.models import DataSource
from typing import Any, Dict, Optional, Sequence
from typing import Any, Dict, Optional, Sequence, Union
from azure.core.credentials import AzureKeyCredential


Expand Down Expand Up @@ -80,20 +82,29 @@ def create_datasource(self, data_source, **kwargs):
def create_or_update_datasource(self, data_source, name=None, **kwargs):
# type: (DataSource, Optional[str], **Any) -> Dict[str, Any]
"""Creates a new datasource or updates a datasource if it already exists.

:param name: The name of the datasource to create or update.
:type name: str
:param data_source: The definition of the datasource to create or update.
:type data_source: ~search.models.DataSource
:keyword match_condition: The match condition to use upon the etag
:type match_condition: ~azure.core.MatchConditions
:return: The created DataSource
:rtype: dict
"""
# TODO: access_condition
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers"))

error_map, access_condition = get_access_conditions(
data_source,
kwargs.pop('match_condition', MatchConditions.Unconditionally)
)
if not name:
name = data_source.name
result = self._client.data_sources.create_or_update(name, data_source, **kwargs)
result = self._client.data_sources.create_or_update(
data_source_name=name,
data_source=data_source,
access_condition=access_condition,
error_map=error_map,
**kwargs
)
return result

@distributed_trace
Expand Down Expand Up @@ -141,13 +152,16 @@ def get_datasources(self, **kwargs):
return result.data_sources

@distributed_trace
def delete_datasource(self, name, **kwargs):
# type: (str, **Any) -> None
"""Deletes a datasource.

:param name: The name of the datasource to delete.
:type name: str

def delete_datasource(self, data_source, **kwargs):
# type: (Union[str, DataSource], **Any) -> None
"""Deletes a datasource. To use access conditions, the Datasource model must be
provided instead of the name. It is enough to provide the name of the datasource
to delete unconditionally

:param data_source: The datasource to delete.
:type data_source: str or ~search.models.DataSource
:keyword match_condition: The match condition to use upon the etag
:type match_condition: ~azure.core.MatchConditions
:return: None
:rtype: None

Expand All @@ -161,4 +175,17 @@ def delete_datasource(self, name, **kwargs):
:caption: Delete a DataSource
"""
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers"))
self._client.data_sources.delete(name, **kwargs)
error_map, access_condition = get_access_conditions(
data_source,
kwargs.pop('match_condition', MatchConditions.Unconditionally)
)
try:
name = data_source.name
except AttributeError:
name = data_source
self._client.data_sources.delete(
data_source_name=name,
access_condition=access_condition,
error_map=error_map,
**kwargs
)
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,23 @@
# --------------------------------------------------------------------------
from typing import TYPE_CHECKING

from azure.core.tracing.decorator import distributed_trace
from azure.core import MatchConditions
from azure.core.exceptions import (
ResourceExistsError,
ResourceNotFoundError,
ResourceModifiedError,
ResourceNotModifiedError,
)
from azure.core.tracing.decorator import distributed_trace
from azure.core.paging import ItemPaged

from ._generated import SearchServiceClient as _SearchServiceClient
from ._generated.models import AccessCondition
from ._utils import (
delistize_flags_for_index,
listize_flags_for_index,
prep_if_match,
prep_if_none_match,
get_access_conditions
)
from .._headers_mixin import HeadersMixin
from .._version import SDK_MONIKER

if TYPE_CHECKING:
# pylint:disable=unused-import,ungrouped-imports
from ._generated.models import AnalyzeRequest, AnalyzeResult, Index
from typing import Any, Dict, List
from typing import Any, Dict, List, Union
from azure.core.credentials import AzureKeyCredential


Expand Down Expand Up @@ -130,12 +122,15 @@ def get_index_statistics(self, index_name, **kwargs):
return result.as_dict()

@distributed_trace
def delete_index(self, index_name, **kwargs):
# type: (str, **Any) -> None
"""Deletes a search index and all the documents it contains.

:param index_name: The name of the index to retrieve.
:type index_name: str
def delete_index(self, index, **kwargs):
# type: (Union[str, Index], **Any) -> None
"""Deletes a search index and all the documents it contains. The model must be
provided instead of the name to use the access conditions.

:param index: The index to retrieve.
:type index: str or ~search.models.Index
:keyword match_condition: The match condition to use upon the etag
:type match_condition: ~azure.core.MatchConditions
:raises: ~azure.core.exceptions.HttpResponseError

.. admonition:: Example:
Expand All @@ -148,7 +143,20 @@ def delete_index(self, index_name, **kwargs):
:caption: Delete an index.
"""
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers"))
self._client.indexes.delete(index_name, **kwargs)
error_map, access_condition = get_access_conditions(
index,
kwargs.pop('match_condition', MatchConditions.Unconditionally)
)
try:
index_name = index.name
except AttributeError:
index_name = index
self._client.indexes.delete(
index_name=index_name,
access_condition=access_condition,
error_map=error_map,
**kwargs
)

@distributed_trace
def create_index(self, index, **kwargs):
Expand Down Expand Up @@ -181,10 +189,9 @@ def create_or_update_index(
index_name,
index,
allow_index_downtime=None,
match_condition=MatchConditions.Unconditionally,
**kwargs
):
# type: (str, Index, bool, MatchConditions, **Any) -> Index
# type: (str, Index, bool, **Any) -> Index
"""Creates a new search index or updates an index if it already exists.

:param index_name: The name of the index.
Expand All @@ -197,7 +204,7 @@ def create_or_update_index(
the index can be impaired for several minutes after the index is updated, or longer for very
large indexes.
:type allow_index_downtime: bool
:param match_condition: The match condition to use upon the etag
:keyword match_condition: The match condition to use upon the etag
:type match_condition: ~azure.core.MatchConditions
:return: The index created or updated
:rtype: :class:`~azure.search.documents.Index`
Expand All @@ -216,29 +223,18 @@ def create_or_update_index(
:dedent: 4
:caption: Update an index.
"""
error_map = {404: ResourceNotFoundError} # type: Dict[int, Any]
access_condition = None
if index.e_tag:
access_condition = AccessCondition()
access_condition.if_match = prep_if_match(index.e_tag, match_condition)
access_condition.if_none_match = prep_if_none_match(
index.e_tag, match_condition
)
if match_condition == MatchConditions.IfNotModified:
error_map[412] = ResourceModifiedError
if match_condition == MatchConditions.IfModified:
error_map[304] = ResourceNotModifiedError
if match_condition == MatchConditions.IfPresent:
error_map[412] = ResourceNotFoundError
if match_condition == MatchConditions.IfMissing:
error_map[412] = ResourceExistsError
error_map, access_condition = get_access_conditions(
index,
kwargs.pop('match_condition', MatchConditions.Unconditionally)
)
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers"))
patched_index = delistize_flags_for_index(index)
result = self._client.indexes.create_or_update(
index_name=index_name,
index=patched_index,
allow_index_downtime=allow_index_downtime,
access_condition=access_condition,
error_map=error_map,
**kwargs
)
return result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@
# --------------------------------------------------------------------------
from typing import TYPE_CHECKING

from azure.core import MatchConditions
from azure.core.tracing.decorator import distributed_trace
from azure.core.exceptions import ClientAuthenticationError, ResourceNotFoundError

from ._generated import SearchServiceClient as _SearchServiceClient
from ._generated.models import Skillset
from ._utils import get_access_conditions
from .._headers_mixin import HeadersMixin
from .._version import SDK_MONIKER

if TYPE_CHECKING:
# pylint:disable=unused-import,ungrouped-imports
from ._generated.models import Skill
from typing import Any, List, Sequence
from typing import Any, List, Sequence, Union
from azure.core.credentials import AzureKeyCredential


Expand Down Expand Up @@ -102,12 +105,16 @@ def get_skillset(self, name, **kwargs):
return self._client.skillsets.get(name, **kwargs)

@distributed_trace
def delete_skillset(self, name, **kwargs):
# type: (str, **Any) -> None
"""Delete a named Skillset in an Azure Search service
def delete_skillset(self, skillset, **kwargs):
# type: (Union[str, Skillset], **Any) -> None
"""Delete a named Skillset in an Azure Search service. To use access conditions,
the Skillset model must be provided instead of the name. It is enough to provide
the name of the skillset to delete unconditionally

:param name: The name of the Skillset to delete
:type name: str
:param name: The Skillset to delete
:type name: str or ~search.models.Skillset
:keyword match_condition: The match condition to use upon the etag
:type match_condition: ~azure.core.MatchConditions

.. admonition:: Example:

Expand All @@ -120,7 +127,15 @@ def delete_skillset(self, name, **kwargs):

"""
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers"))
self._client.skillsets.delete(name, **kwargs)
error_map, access_condition = get_access_conditions(
skillset,
kwargs.pop('match_condition', MatchConditions.Unconditionally)
)
try:
name = skillset.name
except AttributeError:
name = skillset
self._client.skillsets.delete(name, access_condition=access_condition, error_map=error_map, **kwargs)

@distributed_trace
def create_skillset(self, name, skills, description, **kwargs):
Expand Down Expand Up @@ -156,18 +171,19 @@ def create_skillset(self, name, skills, description, **kwargs):
def create_or_update_skillset(self, name, **kwargs):
# type: (str, **Any) -> Skillset
"""Create a new Skillset in an Azure Search service, or update an
existing one.

A `Skillset` object mat
existing one. The skillset param must be provided to perform the
operation with access conditions.

:param name: The name of the Skillset to create or update
:type name: str
:param skills: A list of Skill objects to include in the Skillset
:keyword skills: A list of Skill objects to include in the Skillset
:type skills: List[Skill]
:param description: A description for the Skillset
:keyword description: A description for the Skillset
:type description: Optional[str]
:param skillset: A Skillset to create or update.
:keyword skillset: A Skillset to create or update.
:type skillset: :class:`~azure.search.documents.Skillset`
:keyword match_condition: The match condition to use upon the etag
:type match_condition: ~azure.core.MatchConditions
:return: The created or updated Skillset
:rtype: dict

Expand All @@ -176,9 +192,18 @@ def create_or_update_skillset(self, name, **kwargs):

"""
kwargs["headers"] = self._merge_client_headers(kwargs.get("headers"))
error_map = {
401: ClientAuthenticationError,
404: ResourceNotFoundError
}
access_condition = None

if "skillset" in kwargs:
Copy link
Contributor Author

@rakshith91 rakshith91 Apr 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need more consistency in create_or_update method signatures across clients. We have 3 different ways of implementing them already :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be more specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will drag this discussion off the PR and send a mail about it.

skillset = kwargs.pop("skillset")
error_map, access_condition = get_access_conditions(
skillset,
kwargs.pop('match_condition', MatchConditions.Unconditionally)
)
skillset = Skillset.deserialize(skillset.serialize())
skillset.name = name
for param in ("description", "skills"):
Expand All @@ -192,4 +217,10 @@ def create_or_update_skillset(self, name, **kwargs):
skills=kwargs.pop("skills", None),
)

return self._client.skillsets.create_or_update(name, skillset, **kwargs)
return self._client.skillsets.create_or_update(
skillset_name=name,
skillset=skillset,
access_condition=access_condition,
error_map=error_map,
**kwargs
)
Loading