Conversation
…gly and reran tests
sdk/tables/azure-data-tables/azure/data/tables/_table_client.py
Outdated
Show resolved
Hide resolved
sdk/tables/azure-data-tables/azure/data/tables/_table_client.py
Outdated
Show resolved
Hide resolved
| 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), |
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
The way it currently is if the user passes in None that will be passed to the service.
There was a problem hiding this comment.
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.....
…rrect response instead that it is not None
sdk/tables/azure-data-tables/azure/data/tables/_table_client.py
Outdated
Show resolved
Hide resolved
sdk/tables/azure-data-tables/azure/data/tables/_table_client.py
Outdated
Show resolved
Hide resolved
| try: | ||
| table = self._client.table.create(table_properties) | ||
| return TableItem(table=table) | ||
| metadata, _ = self._client.table.create( |
There was a problem hiding this comment.
What's in this metadata? Do we need to clean out unnecessary information?
There was a problem hiding this comment.
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
sdk/tables/azure-data-tables/azure/data/tables/_table_client.py
Outdated
Show resolved
Hide resolved
sdk/tables/azure-data-tables/azure/data/tables/aio/_table_client_async.py
Outdated
Show resolved
Hide resolved
| partition_key=partition_key, | ||
| row_key=row_key, | ||
| if_match=if_match or if_not_match or '*', | ||
| if_match=if_match or '*', |
There was a problem hiding this comment.
just a question: why are you no longer checking if_not_match
There was a problem hiding this comment.
if_not_match is not supported by the service right now.
| self.assertIn("version", keys) | ||
| self.assertIn("date", keys) | ||
| self.assertIn("etag", keys) | ||
| self.assertEqual(len(keys), 3) |
There was a problem hiding this comment.
do you want to add a test that there's no other metadata besides version, date, and etag? Also wondering how these three were the ones decided on
There was a problem hiding this comment.
There is other metadata but it is intentionally stripped away by @annatisch 's recommendation.
There was a problem hiding this comment.
i see, but can you test that the stripped away metadata isn't present in the final response object, or am I misunderstanding what actually gets stripped away
…into expose_parse_vault_id * 'master' of https://github.com/Azure/azure-sdk-for-python: (37 commits) [text analytics] add versionadded sphinx documentation (Azure#13450) [text analytics] add bing_id property to LinkedEntity class (Azure#13446) fix typing for paging methods (Azure#13410) [text analytics] add domain_filter param (Azure#13451) fix issue Azure#11658 for is_valid_resource_id (Azure#11709) added create_table_if_not_exists method to table service client (Azure#13385) [ServiceBus] Test and failure improvements (Azure#13345) Proper encoding and decoding of source URLs - Fixes special characters in source URL issue (Azure#13275) Switch retry (Azure#13264) [ServiceBus] ServiceBusClient close spawned children (Azure#13077) fixing version issue by not overwriting the version with the semantic… (Azure#13411) clean up reference and tests (Azure#13412) Consistent returns (Azure#13245) [text analytics] return None for offset and length for v3.0 (Azure#13382) edit all authentication files and add a test (Azure#13355) Add managed_identity_client_id argument to DefaultAzureCredential (Azure#13218) [text analytics] add string-index-type support (Azure#13378) [text analytics] fix error response if pii entities is called from v3.0 client (Azure#13383) Send spec (Azure#13143) Anomaly Detector 3.0.0b2 release (Azure#13351) ...
3 participants
Addresses #13239
Creates a consistent response for table client methods. Changing from an entity to a headers response with the eTag