[AppConfig] Address ApiView feedback (#32914)

Author: Yalin Li

sdk/appconfiguration/azure-appconfiguration/CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Bugs Fixed
 
 ### Other Changes
+- Support datetime type for keyword argument `accept_datetime` in `get_snapshot_configuration_settings()`, `list_snapshot_configuration_settings()` and `list_revisions()`.
 
 ## 1.5.0b3 (2023-10-10)
 

sdk/appconfiguration/azure-appconfiguration/assets.json

@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "python",
   "TagPrefix": "python/appconfiguration/azure-appconfiguration",
-  "Tag": "python/appconfiguration/azure-appconfiguration_214d709db6"
+  "Tag": "python/appconfiguration/azure-appconfiguration_e73c991afd"
 }

sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py

@@ -4,6 +4,7 @@
 # license information.
 # -------------------------------------------------------------------------
 import binascii
+from datetime import datetime
 from typing import Any, Dict, List, Mapping, Optional, Union, cast, overload
 from typing_extensions import Literal
 from azure.core import MatchConditions
@@ -41,6 +42,8 @@
     get_endpoint_from_connection_string,
     prep_if_match,
     prep_if_none_match,
+    get_key_filter,
+    get_label_filter,
 )
 from ._sync_token import SyncTokenPolicy
 from ._user_agent import USER_AGENT
@@ -148,7 +151,7 @@ def _create_appconfig_pipeline(self, credential, credential_scopes, aad_mode=Fal
 
     @overload
     def list_configuration_settings(
-        self, *, key_filter: Optional[str] = None, label_filter: Optional[str] = None, **kwargs: Any
+        self, key_filter: Optional[str] = None, label_filter: Optional[str] = None, **kwargs: Any
     ) -> ItemPaged[ConfigurationSetting]:
         """List the configuration settings stored in the configuration service, optionally filtered by
         key, label and accept_datetime.
@@ -159,7 +162,8 @@ def list_configuration_settings(
         :keyword label_filter: filter results based on their label. '*' can be
             used as wildcard in the beginning or end of the filter
         :paramtype label_filter: str
-        :keyword str accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :keyword accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :paramtype accept_datetime: ~datetime.datetime or str
         :keyword list[str] fields: specify which fields to include in the results. Leave None to include all fields
         :return: An iterator of :class:`~azure.appconfiguration.ConfigurationSetting`
         :rtype: ~azure.core.paging.ItemPaged[~azure.appconfiguration.ConfigurationSetting]
@@ -201,7 +205,10 @@ def list_configuration_settings(
         """
 
     @distributed_trace
-    def list_configuration_settings(self, **kwargs) -> ItemPaged[ConfigurationSetting]:
+    def list_configuration_settings(self, *args, **kwargs) -> ItemPaged[ConfigurationSetting]:
+        accept_datetime = kwargs.pop("accept_datetime", None)
+        if isinstance(accept_datetime, datetime):
+            accept_datetime = str(accept_datetime)
         select = kwargs.pop("fields", None)
         if select:
             select = ["locked" if x == "read_only" else x for x in select]
@@ -211,13 +218,17 @@ def list_configuration_settings(self, **kwargs) -> ItemPaged[ConfigurationSettin
             if snapshot_name is not None:
                 return self._impl.get_key_values(  # type: ignore
                     snapshot=snapshot_name,
+                    accept_datetime=accept_datetime,
                     select=select,
                     cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                     **kwargs
                 )
+            key_filter, kwargs = get_key_filter(*args, **kwargs)
+            label_filter, kwargs = get_label_filter(*args, **kwargs)
             return self._impl.get_key_values(  # type: ignore
-                key=kwargs.pop("key_filter", None),
-                label=kwargs.pop("label_filter", None),
+                key=key_filter,
+                label=label_filter,
+                accept_datetime=accept_datetime,
                 select=select,
                 cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                 **kwargs
@@ -244,7 +255,8 @@ def get_configuration_setting(
         :type etag: str or None
         :param match_condition: The match condition to use upon the etag
         :type match_condition: ~azure.core.MatchConditions
-        :keyword str accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :keyword accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :paramtype accept_datetime: ~datetime.datetime or str
         :return: The matched ConfigurationSetting object
         :rtype: ~azure.appconfiguration.ConfigurationSetting or None
         :raises: :class:`~azure.core.exceptions.HttpResponseError`, \
@@ -261,6 +273,10 @@ def get_configuration_setting(
                 key="MyKey", label="MyLabel"
             )
         """
+        accept_datetime = kwargs.pop("accept_datetime", None)
+        if isinstance(accept_datetime, datetime):
+            accept_datetime = str(accept_datetime)
+
         error_map: Dict[int, Any] = {}
         if match_condition == MatchConditions.IfNotModified:
             error_map.update({412: ResourceModifiedError})
@@ -273,6 +289,7 @@ def get_configuration_setting(
             key_value = self._impl.get_key_value(
                 key=key,
                 label=label,
+                accept_datetime=accept_datetime,
                 if_match=prep_if_match(etag, match_condition),
                 if_none_match=prep_if_none_match(etag, match_condition),
                 error_map=error_map,
@@ -460,7 +477,8 @@ def list_revisions(
         :param label_filter: filter results based on their label. '*' can be
             used as wildcard in the beginning or end of the filter
         :type label_filter: str
-        :keyword str accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :keyword accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :paramtype accept_datetime: ~datetime.datetime or str
         :keyword list[str] fields: specify which fields to include in the results. Leave None to include all fields
         :return: An iterator of :class:`~azure.appconfiguration.ConfigurationSetting`
         :rtype: ~azure.core.paging.ItemPaged[~azure.appconfiguration.ConfigurationSetting]
@@ -485,6 +503,9 @@ def list_revisions(
             for item in filtered_revisions:
                 pass  # do something
         """
+        accept_datetime = kwargs.pop("accept_datetime", None)
+        if isinstance(accept_datetime, datetime):
+            accept_datetime = str(accept_datetime)
         select = kwargs.pop("fields", None)
         if select:
             select = ["locked" if x == "read_only" else x for x in select]
@@ -493,6 +514,7 @@ def list_revisions(
             return self._impl.get_revisions(  # type: ignore
                 label=label_filter,
                 key=key_filter,
+                accept_datetime=accept_datetime,
                 select=select,
                 cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                 **kwargs

sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_utils.py

@@ -5,7 +5,7 @@
 # -------------------------------------------------------------------------
 
 from datetime import datetime
-from typing import Optional, Tuple
+from typing import Optional, Tuple, Dict, Any
 from azure.core import MatchConditions
 
 
@@ -70,3 +70,27 @@ def parse_connection_string(connection_string: str) -> Tuple[str, str, str]:
 
 def get_current_utc_time() -> str:
     return str(datetime.utcnow().strftime("%b, %d %Y %H:%M:%S.%f ")) + "GMT"
+
+
+def get_key_filter(*args, **kwargs) -> Tuple[Optional[str], Dict[str, Any]]:
+    key_filter = None
+    if len(args) > 0:
+        key_filter = args[0]
+        if "key_filter" in kwargs:
+            raise TypeError(
+                "AzureAppConfigurationClient.list_configuration_settings() got multiple values for argument "
+                "'key_filter'"
+            )
+    return key_filter or kwargs.pop("key_filter", None), kwargs
+
+
+def get_label_filter(*args, **kwargs) -> Tuple[Optional[str], Dict[str, Any]]:
+    label_filter = None
+    if len(args) > 1:
+        label_filter = args[1]
+        if "label_filter" in kwargs:
+            raise TypeError(
+                "AzureAppConfigurationClient.list_configuration_settings() got multiple values for argument "
+                "'label_filter'"
+            )
+    return label_filter or kwargs.pop("label_filter", None), kwargs

sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py

@@ -4,6 +4,7 @@
 # license information.
 # -------------------------------------------------------------------------
 import binascii
+from datetime import datetime
 from typing import Any, Dict, List, Mapping, Optional, Union, cast, overload
 from typing_extensions import Literal
 from azure.core import MatchConditions
@@ -35,6 +36,8 @@
     get_endpoint_from_connection_string,
     prep_if_match,
     prep_if_none_match,
+    get_key_filter,
+    get_label_filter,
 )
 
 
@@ -122,7 +125,7 @@ def from_connection_string(cls, connection_string: str, **kwargs: Any) -> "Azure
 
     @overload
     def list_configuration_settings(
-        self, *, key_filter: Optional[str] = None, label_filter: Optional[str] = None, **kwargs: Any
+        self, key_filter: Optional[str] = None, label_filter: Optional[str] = None, **kwargs: Any
     ) -> AsyncItemPaged[ConfigurationSetting]:
 
         """List the configuration settings stored in the configuration service, optionally filtered by
@@ -134,7 +137,8 @@ def list_configuration_settings(
         :keyword label_filter: filter results based on their label. '*' can be
             used as wildcard in the beginning or end of the filter
         :paramtype label_filter: str
-        :keyword str accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :keyword accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :paramtype accept_datetime: ~datetime.datetime or str
         :keyword list[str] fields: specify which fields to include in the results. Leave None to include all fields
         :return: An async iterator of :class:`~azure.appconfiguration.ConfigurationSetting`
         :rtype: ~azure.core.async_paging.AsyncItemPaged[~azure.appconfiguration.ConfigurationSetting]
@@ -176,7 +180,11 @@ def list_configuration_settings(
         """
 
     @distributed_trace
-    def list_configuration_settings(self, **kwargs) -> AsyncItemPaged[ConfigurationSetting]:
+    def list_configuration_settings(self, *args, **kwargs) -> AsyncItemPaged[ConfigurationSetting]:
+        accept_datetime = kwargs.pop("accept_datetime", None)
+        if isinstance(accept_datetime, datetime):
+            accept_datetime = str(accept_datetime)
+
         select = kwargs.pop("fields", None)
         if select:
             select = ["locked" if x == "read_only" else x for x in select]
@@ -186,13 +194,17 @@ def list_configuration_settings(self, **kwargs) -> AsyncItemPaged[ConfigurationS
             if snapshot_name is not None:
                 return self._impl.get_key_values(  # type: ignore
                     snapshot=snapshot_name,
+                    accept_datetime=accept_datetime,
                     select=select,
                     cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                     **kwargs
                 )
+            key_filter, kwargs = get_key_filter(*args, **kwargs)
+            label_filter, kwargs = get_label_filter(*args, **kwargs)
             return self._impl.get_key_values(  # type: ignore
-                key=kwargs.pop("key_filter", None),
-                label=kwargs.pop("label_filter", None),
+                key=key_filter,
+                label=label_filter,
+                accept_datetime=accept_datetime,
                 select=select,
                 cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                 **kwargs
@@ -220,7 +232,8 @@ async def get_configuration_setting(
         :type etag: str or None
         :param match_condition: The match condition to use upon the etag
         :type match_condition: ~azure.core.MatchConditions
-        :keyword str accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :keyword accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :paramtype accept_datetime: ~datetime.datetime or str
         :return: The matched ConfigurationSetting object
         :rtype: ~azure.appconfiguration.ConfigurationSetting or None
         :raises: :class:`~azure.core.exceptions.HttpResponseError`, \
@@ -238,6 +251,10 @@ async def get_configuration_setting(
                 key="MyKey", label="MyLabel"
             )
         """
+        accept_datetime = kwargs.pop("accept_datetime", None)
+        if isinstance(accept_datetime, datetime):
+            accept_datetime = str(accept_datetime)
+
         error_map: Dict[int, Any] = {}
         if match_condition == MatchConditions.IfNotModified:
             error_map.update({412: ResourceModifiedError})
@@ -250,6 +267,7 @@ async def get_configuration_setting(
             key_value = await self._impl.get_key_value(
                 key=key,
                 label=label,
+                accept_datetime=accept_datetime,
                 if_match=prep_if_match(etag, match_condition),
                 if_none_match=prep_if_none_match(etag, match_condition),
                 error_map=error_map,
@@ -448,7 +466,8 @@ def list_revisions(
         :param label_filter: filter results based on their label. '*' can be
             used as wildcard in the beginning or end of the filter
         :type label_filter: str
-        :keyword str accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :keyword accept_datetime: retrieve ConfigurationSetting existed at this datetime
+        :paramtype accept_datetime: ~datetime.datetime or str
         :keyword list[str] fields: specify which fields to include in the results. Leave None to include all fields
         :return: An async iterator of :class:`~azure.appconfiguration.ConfigurationSetting`
         :rtype: ~azure.core.async_paging.AsyncItemPaged[~azure.appconfiguration.ConfigurationSetting]
@@ -474,6 +493,9 @@ def list_revisions(
             async for item in filtered_revisions:
                 pass  # do something
         """
+        accept_datetime = kwargs.pop("accept_datetime", None)
+        if isinstance(accept_datetime, datetime):
+            accept_datetime = str(accept_datetime)
         select = kwargs.pop("fields", None)
         if select:
             select = ["locked" if x == "read_only" else x for x in select]
@@ -482,6 +504,7 @@ def list_revisions(
             return self._impl.get_revisions(  # type: ignore
                 label=label_filter,
                 key=key_filter,
+                accept_datetime=accept_datetime,
                 select=select,
                 cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                 **kwargs

sdk/appconfiguration/azure-appconfiguration/tests/test_azure_appconfiguration_client.py

@@ -5,9 +5,9 @@
 # --------------------------------------------------------------------------
 import pytest
 import copy
-import datetime
 import json
 import re
+from datetime import datetime, timezone
 from azure.core import MatchConditions
 from azure.core.exceptions import (
     AzureError,
@@ -222,9 +222,35 @@ def test_delete_configuration_setting_with_etag(self, appconfiguration_connectio
     @recorded_by_proxy
     def test_list_configuration_settings_key_label(self, appconfiguration_connection_string):
         self.set_up(appconfiguration_connection_string)
-        items = list(self.client.list_configuration_settings(label_filter=LABEL, key_filter=KEY))
+        items = list(self.client.list_configuration_settings(KEY, LABEL))
         assert len(items) == 1
         assert all(x.label == LABEL and x.label == LABEL for x in items)
+
+        with pytest.raises(TypeError) as ex:
+            self.client.list_configuration_settings("MyKey1", key_filter="MyKey2")
+        assert (
+            str(ex.value)
+            == "AzureAppConfigurationClient.list_configuration_settings() got multiple values for argument 'key_filter'"
+        )
+        with pytest.raises(TypeError) as ex:
+            self.client.list_configuration_settings("MyKey", "MyLabel1", label_filter="MyLabel2")
+        assert (
+            str(ex.value)
+            == "AzureAppConfigurationClient.list_configuration_settings() got multiple values for argument 'label_filter'"
+        )
+        with pytest.raises(TypeError) as ex:
+            self.client.list_configuration_settings("None", key_filter="MyKey")
+        assert (
+            str(ex.value)
+            == "AzureAppConfigurationClient.list_configuration_settings() got multiple values for argument 'key_filter'"
+        )
+        with pytest.raises(TypeError) as ex:
+            self.client.list_configuration_settings("None", "None", label_filter="MyLabel")
+        assert (
+            str(ex.value)
+            == "AzureAppConfigurationClient.list_configuration_settings() got multiple values for argument 'label_filter'"
+        )
+
         self.tear_down()
 
     @app_config_decorator
@@ -240,7 +266,7 @@ def test_list_configuration_settings_only_label(self, appconfiguration_connectio
     @recorded_by_proxy
     def test_list_configuration_settings_only_key(self, appconfiguration_connection_string):
         self.set_up(appconfiguration_connection_string)
-        items = list(self.client.list_configuration_settings(key_filter=KEY))
+        items = list(self.client.list_configuration_settings(KEY))
         assert len(items) == 2
         assert all(x.key == KEY for x in items)
         self.tear_down()
@@ -336,7 +362,7 @@ def test_list_configuration_settings_no_label(self, appconfiguration_connection_
     @recorded_by_proxy
     def test_list_configuration_settings_only_accepttime(self, appconfiguration_connection_string, **kwargs):
         recorded_variables = kwargs.pop("variables", {})
-        recorded_variables.setdefault("timestamp", str(datetime.datetime.utcnow()))
+        recorded_variables.setdefault("timestamp", str(datetime.utcnow()))
 
         with self.create_client(appconfiguration_connection_string) as client:
             # Confirm all configuration settings are cleaned up
@@ -348,6 +374,10 @@ def test_list_configuration_settings_only_accepttime(self, appconfiguration_conn
             revision = client.list_configuration_settings(accept_datetime=recorded_variables.get("timestamp"))
             assert len(list(revision)) >= 0
 
+            accept_time = datetime(year=2000, month=4, day=1, hour=9, minute=30, second=45, tzinfo=timezone.utc)
+            revision = client.list_configuration_settings(accept_datetime=accept_time)
+            assert len(list(revision)) == 0
+
         return recorded_variables
 
     # method: list_revisions