[AppConfig] Fix a bug for "etag" and Fix pylint-next errors (#33684)

Author: Yalin Li

sdk/appconfiguration/azure-appconfiguration/CHANGELOG.md

@@ -7,6 +7,7 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+- Fixed a bug in consuming "etag" value in sync operation `set_configuration_setting()`.
 
 ### Other Changes
 

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

@@ -210,6 +210,8 @@ def get_configuration_setting(
         label: Optional[str] = None,
         etag: Optional[str] = "*",
         match_condition: MatchConditions = MatchConditions.Unconditionally,
+        *,
+        accept_datetime: Optional[Union[datetime, str]] = None,
         **kwargs,
     ) -> Union[None, ConfigurationSetting]:
         """Get the matched ConfigurationSetting from Azure App Configuration service
@@ -240,7 +242,6 @@ 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)
 
@@ -314,6 +315,8 @@ def set_configuration_setting(
         self,
         configuration_setting: ConfigurationSetting,
         match_condition: MatchConditions = MatchConditions.Unconditionally,
+        *,
+        etag: Optional[str] = None,
         **kwargs,
     ) -> ConfigurationSetting:
         """Add or update a ConfigurationSetting.
@@ -325,7 +328,8 @@ def set_configuration_setting(
         :type configuration_setting: ~azure.appconfiguration.ConfigurationSetting
         :param match_condition: The match condition to use upon the etag
         :type match_condition: ~azure.core.MatchConditions
-        :keyword str etag: check if the ConfigurationSetting is changed. Set None to skip checking etag
+        :keyword str etag: check if the ConfigurationSetting is changed. \
+            Will use the value from param configuration_setting if not set.
         :return: The ConfigurationSetting returned from the service
         :rtype: ~azure.appconfiguration.ConfigurationSetting
         :raises: :class:`~azure.core.exceptions.HttpResponseError`, \
@@ -367,7 +371,7 @@ def set_configuration_setting(
                 key=key_value.key,  # type: ignore
                 label=key_value.label,
                 if_match=prep_if_match(configuration_setting.etag, match_condition),
-                if_none_match=prep_if_none_match(configuration_setting.etag, match_condition),
+                if_none_match=prep_if_none_match(etag or configuration_setting.etag, match_condition),
                 headers=custom_headers,
                 error_map=error_map,
             )
@@ -377,7 +381,13 @@ def set_configuration_setting(
 
     @distributed_trace
     def delete_configuration_setting(  # pylint:disable=delete-operation-wrong-return-type
-        self, key: str, label: Optional[str] = None, **kwargs
+        self,
+        key: str,
+        label: Optional[str] = None,
+        *,
+        etag: Optional[str] = None,
+        match_condition: MatchConditions = MatchConditions.Unconditionally,
+        **kwargs,
     ) -> ConfigurationSetting:
         """Delete a ConfigurationSetting if it exists
 
@@ -406,8 +416,6 @@ def delete_configuration_setting(  # pylint:disable=delete-operation-wrong-retur
                 key="MyKey", label="MyLabel"
             )
         """
-        etag = kwargs.pop("etag", None)
-        match_condition = kwargs.pop("match_condition", MatchConditions.Unconditionally)
         custom_headers: Mapping[str, Any] = CaseInsensitiveDict(kwargs.get("headers"))
         error_map: Dict[int, Any] = {409: ResourceReadOnlyError}
         if match_condition == MatchConditions.IfNotModified:
@@ -433,7 +441,13 @@ def delete_configuration_setting(  # pylint:disable=delete-operation-wrong-retur
 
     @distributed_trace
     def list_revisions(
-        self, key_filter: Optional[str] = None, label_filter: Optional[str] = None, **kwargs
+        self,
+        key_filter: Optional[str] = None,
+        label_filter: Optional[str] = None,
+        *,
+        accept_datetime: Optional[Union[datetime, str]] = None,
+        fields: Optional[List[str]] = None,
+        **kwargs,
     ) -> ItemPaged[ConfigurationSetting]:
         """
         Find the ConfigurationSetting revision history, optionally filtered by key, label and accept_datetime.
@@ -470,19 +484,17 @@ 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]
+        if fields:
+            fields = ["locked" if x == "read_only" else x for x in fields]
 
         try:
             return self._impl.get_revisions(  # type: ignore
                 label=label_filter,
                 key=key_filter,
                 accept_datetime=accept_datetime,
-                select=select,
+                select=fields,
                 cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                 **kwargs,
             )
@@ -491,7 +503,12 @@ def list_revisions(
 
     @distributed_trace
     def set_read_only(
-        self, configuration_setting: ConfigurationSetting, read_only: bool = True, **kwargs
+        self,
+        configuration_setting: ConfigurationSetting,
+        read_only: bool = True,
+        *,
+        match_condition: MatchConditions = MatchConditions.Unconditionally,
+        **kwargs,
     ) -> ConfigurationSetting:
         """Set a configuration setting read only
 
@@ -519,7 +536,6 @@ def set_read_only(
             read_only_config_setting = client.set_read_only(config_setting, read_only=False)
         """
         error_map: Dict[int, Any] = {}
-        match_condition = kwargs.pop("match_condition", MatchConditions.Unconditionally)
         if match_condition == MatchConditions.IfNotModified:
             error_map.update({412: ResourceModifiedError})
         if match_condition == MatchConditions.IfModified:

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

@@ -218,6 +218,8 @@ async def get_configuration_setting(
         label: Optional[str] = None,
         etag: Optional[str] = "*",
         match_condition: MatchConditions = MatchConditions.Unconditionally,
+        *,
+        accept_datetime: Optional[Union[datetime, str]] = None,
         **kwargs,
     ) -> Union[None, ConfigurationSetting]:
 
@@ -250,7 +252,6 @@ 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)
 
@@ -329,6 +330,8 @@ async def set_configuration_setting(
         self,
         configuration_setting: ConfigurationSetting,
         match_condition: MatchConditions = MatchConditions.Unconditionally,
+        *,
+        etag: Optional[str] = None,
         **kwargs,
     ) -> ConfigurationSetting:
 
@@ -341,7 +344,8 @@ async def set_configuration_setting(
         :type configuration_setting: ~azure.appconfiguration.ConfigurationSetting
         :param match_condition: The match condition to use upon the etag
         :type match_condition: ~azure.core.MatchConditions
-        :keyword str etag: check if the ConfigurationSetting is changed. Set None to skip checking etag
+        :keyword str etag: check if the ConfigurationSetting is changed. \
+            Will use the value from param configuration_setting if not set.
         :return: The ConfigurationSetting returned from the service
         :rtype: ~azure.appconfiguration.ConfigurationSetting
         :raises: :class:`~azure.core.exceptions.HttpResponseError`, \
@@ -366,8 +370,6 @@ async def set_configuration_setting(
             )
             returned_config_setting = await async_client.set_configuration_setting(config_setting)
         """
-        etag = kwargs.get("etag", configuration_setting.etag)
-
         key_value = configuration_setting._to_generated()
         custom_headers: Mapping[str, Any] = CaseInsensitiveDict(kwargs.get("headers"))
         error_map: Dict[int, Any] = {409: ResourceReadOnlyError}
@@ -386,7 +388,7 @@ async def set_configuration_setting(
                 key=key_value.key,  # type: ignore
                 label=key_value.label,
                 if_match=prep_if_match(configuration_setting.etag, match_condition),
-                if_none_match=prep_if_none_match(etag, match_condition),
+                if_none_match=prep_if_none_match(etag or configuration_setting.etag, match_condition),
                 headers=custom_headers,
                 error_map=error_map,
             )
@@ -396,7 +398,13 @@ async def set_configuration_setting(
 
     @distributed_trace_async
     async def delete_configuration_setting(
-        self, key: str, label: Optional[str] = None, **kwargs
+        self,
+        key: str,
+        label: Optional[str] = None,
+        *,
+        etag: Optional[str] = None,
+        match_condition: MatchConditions = MatchConditions.Unconditionally,
+        **kwargs,
     ) -> ConfigurationSetting:
         """Delete a ConfigurationSetting if it exists
 
@@ -426,8 +434,6 @@ async def delete_configuration_setting(
                 key="MyKey", label="MyLabel"
             )
         """
-        etag = kwargs.pop("etag", None)
-        match_condition = kwargs.pop("match_condition", MatchConditions.Unconditionally)
         custom_headers: Mapping[str, Any] = CaseInsensitiveDict(kwargs.get("headers"))
         error_map: Dict[int, Any] = {409: ResourceReadOnlyError}
         if match_condition == MatchConditions.IfNotModified:
@@ -453,7 +459,13 @@ async def delete_configuration_setting(
 
     @distributed_trace
     def list_revisions(
-        self, key_filter: Optional[str] = None, label_filter: Optional[str] = None, **kwargs
+        self,
+        key_filter: Optional[str] = None,
+        label_filter: Optional[str] = None,
+        *,
+        accept_datetime: Optional[Union[datetime, str]] = None,
+        fields: Optional[List[str]] = None,
+        **kwargs,
     ) -> AsyncItemPaged[ConfigurationSetting]:
 
         """
@@ -492,19 +504,17 @@ 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]
+        if fields:
+            fields = ["locked" if x == "read_only" else x for x in fields]
 
         try:
             return self._impl.get_revisions(  # type: ignore
                 label=label_filter,
                 key=key_filter,
                 accept_datetime=accept_datetime,
-                select=select,
+                select=fields,
                 cls=lambda objs: [ConfigurationSetting._from_generated(x) for x in objs],
                 **kwargs,
             )
@@ -513,7 +523,12 @@ def list_revisions(
 
     @distributed_trace_async
     async def set_read_only(
-        self, configuration_setting: ConfigurationSetting, read_only: bool = True, **kwargs
+        self,
+        configuration_setting: ConfigurationSetting,
+        read_only: bool = True,
+        *,
+        match_condition: MatchConditions = MatchConditions.Unconditionally,
+        **kwargs,
     ) -> ConfigurationSetting:
 
         """Set a configuration setting read only
@@ -542,7 +557,6 @@ async def set_read_only(
             read_only_config_setting = await client.set_read_only(config_setting, read_only=False)
         """
         error_map: Dict[int, Any] = {}
-        match_condition = kwargs.pop("match_condition", MatchConditions.Unconditionally)
         if match_condition == MatchConditions.IfNotModified:
             error_map.update({412: ResourceModifiedError})
         if match_condition == MatchConditions.IfModified: