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.

Sign up for GitHub

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

Jump to bottom

Consistent returns #13245

Merged
merged 17 commits into from
Aug 28, 2020
Merged

Consistent returns #13245

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ccafe0f
fixes sync create_entity to return just the metadata
seankane-msft Aug 20, 2020
4b1f75a
fixes the create_table method to return just metadata
seankane-msft Aug 20, 2020
c21fd4c
fixes the create_table method to return just metadata
seankane-msft Aug 20, 2020
20cb21a
fixed update_entity too
seankane-msft Aug 20, 2020
e4de1d5
changed upsert_entity too, now to change the async methods
seankane-msft Aug 20, 2020
d7f1f00
fixed async create and update entity
seankane-msft Aug 20, 2020
7be28fc
fixed async create and update entity
seankane-msft Aug 20, 2020
94b7415
updated async methods to return metadata only, updated tests accordin…
seankane-msft Aug 20, 2020
ba82b25
tox fixes
seankane-msft Aug 20, 2020
1499b47
fixing merge conflicts
seankane-msft Aug 20, 2020
2a71664
merge conflicts round 2
seankane-msft Aug 21, 2020
56bfcdd
addressing izzys comments, fixing typehint formats, checking for a co…
seankane-msft Aug 24, 2020
1b0a191
fixed comments
seankane-msft Aug 24, 2020
1d52074
making the same change for async code
seankane-msft Aug 24, 2020
ade3d40
addressing annas comments, trimming metadata returns, removed if_not_…
seankane-msft Aug 25, 2020
c10926d
had an extra space in there causing pylint failure
seankane-msft Aug 25, 2020
49c0137
changed how _trim_service_metadata works
seankane-msft Aug 25, 2020
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
74 changes: 43 additions & 31 deletions sdk/tables/azure-data-tables/azure/data/tables/_table_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from ._deserialize import _return_headers_and_deserialized
from ._error import _process_table_error
from ._version import VERSION
from ._models import TableEntityPropertiesPaged, UpdateMode, TableItem
from ._models import TableEntityPropertiesPaged, UpdateMode


class TableClient(TableClientBase):
Expand Down Expand Up @@ -180,17 +180,19 @@ def create_table(
self,
**kwargs # type: Any
):
# type: (...) -> TableItem
# type: (...) -> Dict[str,str]
"""Creates a new table under the current account.

:return: TableItem created
:rtype: TableItem
:return: Dictionary of response headers from service
:rtype: Dict[str,str]
:raises: ~azure.core.exceptions.HttpResponseError
"""
table_properties = TableProperties(table_name=self.table_name, **kwargs)
try:
table = self._client.table.create(table_properties)
return TableItem(table=table)
metadata, _ = self._client.table.create(
Copy link
Member

Choose a reason for hiding this comment

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

What's in this metadata? Do we need to clean out unnecessary information?

Copy link
Member Author

Choose a reason for hiding this comment

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

The returned metadata has these keys: client_request_id, request_id, version, date, and etag always. On create_entity the service also returns preference_applied and content_type. client_request_id, request_id, and preference_applied are all None in the tests

table_properties,
cls=kwargs.pop('cls', _return_headers_and_deserialized))
return metadata
except HttpResponseError as error:
_process_table_error(error)

Expand Down Expand Up @@ -250,29 +252,27 @@ def create_entity(
entity, # type: Union[TableEntity, dict[str,str]]
**kwargs # type: Any
):
# type: (...) -> TableEntity
# type: (...) -> dict[str,str]
seankane-msft marked this conversation as resolved.
Show resolved Hide resolved
"""Insert entity in a table.

:param entity: The properties for the table entity.
:type entity: Union[TableEntity, dict[str,str]]
:return: TableEntity mapping str to azure.data.tables.EntityProperty
:rtype: ~azure.data.tables.TableEntity
:return: Dictionary mapping response headers from the service
seankane-msft marked this conversation as resolved.
Show resolved Hide resolved
:rtype: dict[str,str]
:raises: ~azure.core.exceptions.HttpResponseError
"""

if "PartitionKey" in entity and "RowKey" in entity:
entity = _add_entity_properties(entity)
# TODO: Remove - and run test to see what happens with the service
else:
raise ValueError('PartitionKey and RowKey were not provided in entity')
try:
inserted_entity = self._client.table.insert_entity(
metadata, _ = self._client.table.insert_entity(
table=self.table_name,
table_entity_properties=entity,
**kwargs
)
properties = _convert_to_entity(inserted_entity)
return properties
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs)
return metadata
except ResourceNotFoundError as error:
_process_table_error(error)

Expand All @@ -283,7 +283,7 @@ def update_entity( # pylint:disable=R1710
mode=UpdateMode.MERGE, # type: UpdateMode
**kwargs # type: Any
):
# type: (...) -> None
# type: (...) -> Dict[str,str]
"""Update entity in a table.

:param entity: The properties for the table entity.
Expand All @@ -294,8 +294,8 @@ def update_entity( # pylint:disable=R1710
:keyword str row_key: The row key of the entity.
:keyword str etag: Etag of the entity
:keyword ~azure.core.MatchConditions match_condition: MatchCondition
:return: None
:rtype: None
:return: Dictionary mapping response headers from the service
:rtype: dict[str,str
seankane-msft marked this conversation as resolved.
Show resolved Hide resolved
:raises: ~azure.core.exceptions.HttpResponseError
"""

Expand All @@ -307,20 +307,28 @@ def update_entity( # pylint:disable=R1710
row_key = entity['RowKey']
entity = _add_entity_properties(entity)
try:
metadata = None
if mode is UpdateMode.REPLACE:
self._client.table.update_entity(
metadata, _ = self._client.table.update_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
if_match=if_match or if_not_match or "*",
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs)
elif mode is UpdateMode.MERGE:
self._client.table.merge_entity(table=self.table_name, partition_key=partition_key,
row_key=row_key, if_match=if_match or if_not_match or "*",
table_entity_properties=entity, **kwargs)
metadata, _ = self._client.table.merge_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
if_match=if_match or if_not_match or "*",
table_entity_properties=entity,
cls=kwargs.pop('cls', _return_headers_and_deserialized),
Copy link
Contributor

Choose a reason for hiding this comment

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

@annatisch had mentioned before that she prefers something more like
cls=kwargs.pop('cls', None) or _return_headers_and_deserialized (can't remember this comment correctly, I think it might have had something to do with if the user passes in None for cls

Copy link
Member Author

Choose a reason for hiding this comment

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

The way it currently is if the user passes in None that will be passed to the service.

Copy link
Member

Choose a reason for hiding this comment

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

It wont be passed to the service - as this parameter deals with client-side behaviour.
But yeah... this one is tricky, because if a user does provide a cls parameter, and it doesn't return a tuple of two values - the function will fail.....

**kwargs)
else:
raise ValueError('Mode type is not supported')
return metadata
except HttpResponseError as error:
_process_table_error(error)

Expand Down Expand Up @@ -395,15 +403,15 @@ def get_entity(
row_key, # type: str
**kwargs # type: Any
):
# type: (...) -> TableEntity
# type: (...) -> Dict[str,str]
"""Queries entities in a table.

:param partition_key: The partition key of the entity.
:type partition_key: str
:param row_key: The row key of the entity.
:type row_key: str
:return: Entity mapping str to azure.data.tables.EntityProperty
:rtype: ~azure.data.tables.TableEntity
:return: Dictionary mapping response headers from the service
:rtype: dict[str,str
seankane-msft marked this conversation as resolved.
Show resolved Hide resolved
:raises: ~azure.core.exceptions.HttpResponseError
"""
try:
Expand All @@ -424,15 +432,15 @@ def upsert_entity( # pylint:disable=R1710
mode=UpdateMode.MERGE, # type: UpdateMode
**kwargs # type: Any
):
# type: (...) -> None
# type: (...) -> Dict[str,str]
"""Update/Merge or Insert entity into table.

:param entity: The properties for the table entity.
:type entity: Union[TableEntity, dict[str,str]]
:param mode: Merge or Replace and Insert on fail
:type mode: ~azure.data.tables.UpdateMode
:return: None
:rtype: None
:return: Dictionary mapping response headers from the service
:rtype: dict[str,str
:raises: ~azure.core.exceptions.HttpResponseError
"""

Expand All @@ -441,25 +449,29 @@ def upsert_entity( # pylint:disable=R1710
entity = _add_entity_properties(entity)

try:
metadata = None
if mode is UpdateMode.MERGE:
self._client.table.merge_entity(
metadata, _ = self._client.table.merge_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs
)
elif mode is UpdateMode.REPLACE:
self._client.table.update_entity(
metadata, _ = self._client.table.update_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs)
else:
raise ValueError('Mode type is not supported')
seankane-msft marked this conversation as resolved.
Show resolved Hide resolved
return metadata
except ResourceNotFoundError:
self.create_entity(
return self.create_entity(
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
Expand Down
73 changes: 42 additions & 31 deletions sdk/tables/azure-data-tables/azure/data/tables/aio/_table_client_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from .._entity import TableEntity
from .._generated.aio import AzureTable
from .._generated.models import SignedIdentifier, TableProperties, QueryOptions
from .._models import AccessPolicy, TableItem
from .._models import AccessPolicy
from .._serialize import serialize_iso
from .._deserialize import _return_headers_and_deserialized
from .._error import _process_table_error
Expand Down Expand Up @@ -193,16 +193,18 @@ async def create_table(
self,
**kwargs # type: Any
):
# type: (...) -> TableItem
# type: (...) -> Dict[str,str]
"""Creates a new table under the given account.
:return: Table created
:rtype: TableItem
:return: Dictionary of response headers from service
:rtype: Dict[str,str]
:raises: ~azure.core.exceptions.HttpResponseError
"""
table_properties = TableProperties(table_name=self.table_name, **kwargs)
try:
table = await self._client.table.create(table_properties)
return TableItem(table)
metadata, _ = await self._client.table.create(
table_properties,
cls=kwargs.pop('cls', _return_headers_and_deserialized))
return metadata
except HttpResponseError as error:
_process_table_error(error)

Expand Down Expand Up @@ -259,28 +261,26 @@ async def create_entity(
entity, # type: Union[TableEntity, dict[str,str]]
**kwargs # type: Any
):
# type: (...) -> TableEntity
# type: (...) -> Dict[str,str]
"""Insert entity in a table.
:param entity: The properties for the table entity.
:type entity: dict[str, str]
:return: TableEntity mapping str to azure.data.tables.EntityProperty
:rtype: ~azure.data.tables.TableEntity
:return: Dictionary of response headers from service
:rtype: Dict[str,str]
:raises: ~azure.core.exceptions.HttpResponseError
"""

if entity:
if "PartitionKey" in entity and "RowKey" in entity:
entity = _add_entity_properties(entity)
else:
raise ValueError('PartitionKey and RowKey were not provided in entity')
if "PartitionKey" in entity and "RowKey" in entity:
entity = _add_entity_properties(entity)
else:
raise ValueError('PartitionKey and RowKey were not provided in entity')
try:
inserted_entity = await self._client.table.insert_entity(
metadata, _ = await self._client.table.insert_entity(
table=self.table_name,
table_entity_properties=entity,
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs
)
properties = _convert_to_entity(inserted_entity)
return properties
return metadata
except ResourceNotFoundError as error:
_process_table_error(error)

Expand All @@ -291,7 +291,7 @@ async def update_entity(
mode=UpdateMode.MERGE, # type: UpdateMode
**kwargs # type: Any
):
# type: (...) -> None
# type: (...) -> Dict[str,str]
"""Update entity in a table.
:param mode: Merge or Replace entity
:type mode: ~azure.data.tables.UpdateMode
Expand All @@ -305,8 +305,8 @@ async def update_entity(
:type etag: str
:param match_condition: MatchCondition
:type match_condition: ~azure.core.MatchConditions
:return: None
:rtype: None
:return: Dictionary of response headers from service
:rtype: Dict[str,str]
:raises: ~azure.core.exceptions.HttpResponseError
"""
if_match, if_not_match = _get_match_headers(kwargs=dict(kwargs, etag=kwargs.pop('etag', None),
Expand All @@ -317,20 +317,27 @@ async def update_entity(
row_key = entity['RowKey']
entity = _add_entity_properties(entity)
try:
metadata = None
if mode is UpdateMode.REPLACE:
await self._client.table.update_entity(
metadata, _ = await self._client.table.update_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
if_match=if_match or if_not_match or "*",
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs)
elif mode is UpdateMode.MERGE:
await self._client.table.merge_entity(table=self.table_name, partition_key=partition_key,
row_key=row_key, if_match=if_match or if_not_match or "*",
table_entity_properties=entity, **kwargs)
metadata, _ = await self._client.table.merge_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
if_match=if_match or if_not_match or "*",
seankane-msft marked this conversation as resolved.
Show resolved Hide resolved
cls=kwargs.pop('cls', _return_headers_and_deserialized),
table_entity_properties=entity, **kwargs)
else:
raise ValueError('Mode type is not supported')
return metadata
except HttpResponseError as error:
_process_table_error(error)

Expand Down Expand Up @@ -433,15 +440,15 @@ async def upsert_entity(
mode=UpdateMode.MERGE, # type: UpdateMode
**kwargs # type: Any
):
# type: (...) -> None
# type: (...) -> Dict[str,str]

"""Update/Merge or Insert entity into table.
:param mode: Merge or Replace and Insert on fail
:type mode: ~azure.data.tables.UpdateMode
:param entity: The properties for the table entity.
:type entity: dict[str, str]
:return: Entity mapping str to azure.data.tables.EntityProperty or None
:rtype: None
:return: Dictionary of response headers from service
:rtype: Dict[str,str]
:raises: ~azure.core.exceptions.HttpResponseError
"""

Expand All @@ -450,25 +457,29 @@ async def upsert_entity(
entity = _add_entity_properties(entity)

try:
metadata = None
if mode is UpdateMode.MERGE:
await self._client.table.merge_entity(
metadata, _ = await self._client.table.merge_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs
)
elif mode is UpdateMode.REPLACE:
await self._client.table.update_entity(
metadata, _ = await self._client.table.update_entity(
table=self.table_name,
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
cls=kwargs.pop('cls', _return_headers_and_deserialized),
**kwargs)
else:
raise ValueError('Mode type is not supported')
return metadata
except ResourceNotFoundError:
await self.create_entity(
return await self.create_entity(
partition_key=partition_key,
row_key=row_key,
table_entity_properties=entity,
Expand Down
Loading