Skip to content

Commit c2e72e4

Browse files
seligj95Copilot
andcommitted
Address review comments and fix style: slot_cfg reconciliation, stronger test assertions
- Fix pylint R1735: use dict literal instead of dict() call (line 6009) - In replace_all mode, reconcile slot_cfg_names.connection_string_names to exactly match new slotSetting=true entries, removing stale names - Strengthen test_slot_param_configured_with_id_part to check that the slot argument specifically (not just any argument) has id_part - Fix TestListSslCertsPagination docstring: remove incorrect Issue #29495 reference since that issue is about az webapp list, not SSL cert pagination Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9f725de commit c2e72e4

2 files changed

Lines changed: 27 additions & 7 deletions

File tree

src/azure-cli/azure/cli/command_modules/appservice/custom.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3569,7 +3569,13 @@ def update_connection_strings(cmd, resource_group_name, name, connection_string_
35693569
'update_connection_strings',
35703570
conn_strings, slot, client)
35713571

3572-
if sticky_slot_settings or rm_sticky_slot_settings:
3572+
if replace_all:
3573+
# Replace-all: slot config names must exactly match new slotSetting=true entries
3574+
new_slot_setting_names = set(n['name'] for n in sticky_slot_settings)
3575+
slot_cfg_names = client.web_apps.list_slot_configuration_names(resource_group_name, name)
3576+
slot_cfg_names.connection_string_names = list(new_slot_setting_names)
3577+
client.web_apps.update_slot_configuration_names(resource_group_name, name, slot_cfg_names)
3578+
elif sticky_slot_settings or rm_sticky_slot_settings:
35733579
new_slot_setting_names = set(n['name'] for n in sticky_slot_settings) # add setting name
35743580
slot_cfg_names = client.web_apps.list_slot_configuration_names(resource_group_name, name)
35753581
slot_cfg_names.connection_string_names = set(slot_cfg_names.connection_string_names or [])
@@ -6006,8 +6012,8 @@ def create_managed_ssl_cert(cmd, resource_group_name, name, hostname, slot=None,
60066012

60076013
server_farm_id = webapp.server_farm_id
60086014
location = webapp.location
6009-
cert_kwargs = dict(location=location, canonical_name=hostname,
6010-
server_farm_id=server_farm_id, password='')
6015+
cert_kwargs = {"location": location, "canonical_name": hostname,
6016+
"server_farm_id": server_farm_id, "password": ''}
60116017
if domain_validation_method:
60126018
cert_kwargs['domain_validation_method'] = domain_validation_method
60136019
easy_cert_def = Certificate(**cert_kwargs)

src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,11 @@ def test_json_settings_replaces_all(self, mock_redact, mock_client_factory, mock
779779
client_mock = mock.MagicMock()
780780
mock_client_factory.return_value = client_mock
781781

782+
# Simulate stale slot config names for connection strings that will be removed
783+
slot_cfg_mock = mock.MagicMock()
784+
slot_cfg_mock.connection_string_names = ['OldConn', 'KeepConn']
785+
client_mock.web_apps.list_slot_configuration_names.return_value = slot_cfg_mock
786+
782787
json_settings = ['[{"name":"NewConn","value":"new_val","type":"SQLAzure"}]']
783788
update_connection_strings(cmd_mock, 'rg', 'app', settings=json_settings)
784789

@@ -787,6 +792,11 @@ def test_json_settings_replaces_all(self, mock_redact, mock_client_factory, mock
787792
self.assertNotIn('KeepConn', existing_conn_strings.properties)
788793
self.assertIn('NewConn', existing_conn_strings.properties)
789794

795+
# Stale slot config names should be cleared in replace-all mode
796+
client_mock.web_apps.update_slot_configuration_names.assert_called_once()
797+
updated_cfg = client_mock.web_apps.update_slot_configuration_names.call_args[0][2]
798+
self.assertEqual(updated_cfg.connection_string_names, [])
799+
790800
@mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation')
791801
@mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation')
792802
@mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory')
@@ -819,7 +829,7 @@ def test_key_value_settings_merges(self, mock_redact, mock_client_factory, mock_
819829

820830

821831
class TestListSslCertsPagination(unittest.TestCase):
822-
"""Tests for list_ssl_certs pagination fix (Issue #29495)."""
832+
"""Tests for list_ssl_certs pagination behavior."""
823833

824834
@mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory')
825835
def test_list_ssl_certs_returns_list(self, mock_client_factory):
@@ -875,9 +885,13 @@ def test_slot_param_configured_with_id_part(self):
875885
from azure.cli.command_modules.appservice import _params
876886

877887
source = inspect.getsource(_params.load_arguments)
878-
# The webapp context should have slot configured with id_part='child_name_1'
879-
self.assertIn("id_part='child_name_1'", source,
880-
"slot argument must have id_part='child_name_1' for --ids slot parsing")
888+
# Find the specific line that configures the slot argument with id_part
889+
slot_lines = [line.strip() for line in source.splitlines()
890+
if "argument('slot'" in line and "id_part=" in line]
891+
self.assertTrue(len(slot_lines) > 0,
892+
"Expected c.argument('slot', ..., id_part=...) in load_arguments")
893+
self.assertTrue(any("child_name_1" in line for line in slot_lines),
894+
"slot argument must have id_part='child_name_1' for --ids slot parsing")
881895

882896

883897
class FakedResponse: # pylint: disable=too-few-public-methods

0 commit comments

Comments
 (0)