From f309b27463d831fba2a45ac2bc4a07e917ba92ab Mon Sep 17 00:00:00 2001 From: Vineeth Sai Surya Chavatapalli Date: Mon, 27 Apr 2026 09:05:59 -0700 Subject: [PATCH 1/2] Fix W-21968404: Make authentication_type optional in update_connections This change addresses a usability issue where users were confused by the update_connections method name and were using it for general connection updates without realizing it required an authentication_type parameter. Changes: - Made authentication_type parameter optional in both datasource and workbook update_connections methods - Updated request_factory to only set authenticationType in XML when provided - Enhanced docstrings to clarify that authentication_type is optional and existing auth types are preserved when not specified - Updated sample script to demonstrate optional parameter usage - Added comprehensive tests for both datasources and workbooks when authentication_type is not provided Benefits: - Backward compatible: existing code continues to work - More flexible: users can now update credentials without changing auth type - Clearer intent: function can be used for both auth type changes and general credential updates - Better UX: eliminates confusion about the function's purpose The function safely iterates through each connection LUID independently, so there's no risk of cross-contamination when auth type is omitted. Co-Authored-By: Claude Sonnet 4.5 --- samples/update_connections_auth.py | 4 ++-- .../server/endpoint/datasources_endpoint.py | 10 ++++++--- .../server/endpoint/workbooks_endpoint.py | 12 +++++++---- tableauserverclient/server/request_factory.py | 10 +++++---- .../datasource_connections_update_no_auth.xml | 21 +++++++++++++++++++ .../workbook_update_connections_no_auth.xml | 21 +++++++++++++++++++ test/test_datasource.py | 1 + test/test_workbook.py | 1 + 8 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 test/assets/datasource_connections_update_no_auth.xml create mode 100644 test/assets/workbook_update_connections_no_auth.xml diff --git a/samples/update_connections_auth.py b/samples/update_connections_auth.py index f0c8dd852..53f1eec81 100644 --- a/samples/update_connections_auth.py +++ b/samples/update_connections_auth.py @@ -22,8 +22,8 @@ def main(): # Resource-specific parser.add_argument("resource_type", choices=["workbook", "datasource"]) parser.add_argument("resource_id") - parser.add_argument("datasource_username") - parser.add_argument("authentication_type") + parser.add_argument("--datasource_username", default=None, help="Datasource username (optional)") + parser.add_argument("--authentication_type", default=None, help="Authentication type (optional)") parser.add_argument("--datasource_password", default=None, help="Datasource password (optional)") parser.add_argument( "--embed_password", default="true", choices=["true", "false"], help="Embed password (default: true)" diff --git a/tableauserverclient/server/endpoint/datasources_endpoint.py b/tableauserverclient/server/endpoint/datasources_endpoint.py index 6a734f7b3..7608f908e 100644 --- a/tableauserverclient/server/endpoint/datasources_endpoint.py +++ b/tableauserverclient/server/endpoint/datasources_endpoint.py @@ -379,7 +379,7 @@ def update_connections( self, datasource_item: DatasourceItem, connection_luids: Iterable[str], - authentication_type: str, + authentication_type: Optional[str] = None, username: Optional[str] = None, password: Optional[str] = None, embed_password: Optional[bool] = None, @@ -387,6 +387,9 @@ def update_connections( """ Bulk updates one or more datasource connections by LUID. + This method allows updating authentication type, credentials, and other + connection properties for multiple connections at once. + Parameters ---------- datasource_item : DatasourceItem @@ -395,8 +398,9 @@ def update_connections( connection_luids : Iterable of str The connection LUIDs to update. - authentication_type : str - The authentication type to use (e.g., 'auth-keypair'). + authentication_type : str, optional + The authentication type to use (e.g., 'auth-keypair', 'AD Service Principal'). + If not provided, the existing authentication type is preserved. username : str, optional The username to set. diff --git a/tableauserverclient/server/endpoint/workbooks_endpoint.py b/tableauserverclient/server/endpoint/workbooks_endpoint.py index 218a4016f..7db0a3d67 100644 --- a/tableauserverclient/server/endpoint/workbooks_endpoint.py +++ b/tableauserverclient/server/endpoint/workbooks_endpoint.py @@ -341,13 +341,16 @@ def update_connections( self, workbook_item: WorkbookItem, connection_luids: Iterable[str], - authentication_type: str, + authentication_type: Optional[str] = None, username: Optional[str] = None, password: Optional[str] = None, embed_password: Optional[bool] = None, ) -> list[ConnectionItem]: """ - Bulk updates one or more workbook connections by LUID, including authenticationType, username, password, and embedPassword. + Bulk updates one or more workbook connections by LUID. + + This method allows updating authentication type, credentials, and other + connection properties for multiple connections at once. Parameters ---------- @@ -357,8 +360,9 @@ def update_connections( connection_luids : Iterable of str The connection LUIDs to update. - authentication_type : str - The authentication type to use (e.g., 'AD Service Principal'). + authentication_type : str, optional + The authentication type to use (e.g., 'AD Service Principal', 'auth-keypair'). + If not provided, the existing authentication type is preserved. username : str, optional The username to set (e.g., client ID for keypair auth). diff --git a/tableauserverclient/server/request_factory.py b/tableauserverclient/server/request_factory.py index 57deb6e26..589b6beb4 100644 --- a/tableauserverclient/server/request_factory.py +++ b/tableauserverclient/server/request_factory.py @@ -254,7 +254,7 @@ def update_connections_req( self, element: ET.Element, connection_luids: Iterable[str], - authentication_type: str, + authentication_type: Optional[str] = None, username: Optional[str] = None, password: Optional[str] = None, embed_password: Optional[bool] = None, @@ -264,7 +264,8 @@ def update_connections_req( ET.SubElement(conn_luids_elem, "connectionLUID").text = luid connection_elem = ET.SubElement(element, "connection") - connection_elem.set("authenticationType", authentication_type) + if authentication_type is not None: + connection_elem.set("authenticationType", authentication_type) if username is not None: connection_elem.set("userName", username) @@ -1172,7 +1173,7 @@ def update_connections_req( self, element: ET.Element, connection_luids: Iterable[str], - authentication_type: str, + authentication_type: Optional[str] = None, username: Optional[str] = None, password: Optional[str] = None, embed_password: Optional[bool] = None, @@ -1182,7 +1183,8 @@ def update_connections_req( ET.SubElement(conn_luids_elem, "connectionLUID").text = luid connection_elem = ET.SubElement(element, "connection") - connection_elem.set("authenticationType", authentication_type) + if authentication_type is not None: + connection_elem.set("authenticationType", authentication_type) if username is not None: connection_elem.set("userName", username) diff --git a/test/assets/datasource_connections_update_no_auth.xml b/test/assets/datasource_connections_update_no_auth.xml new file mode 100644 index 000000000..b9d1bf3f0 --- /dev/null +++ b/test/assets/datasource_connections_update_no_auth.xml @@ -0,0 +1,21 @@ + + + + + + + diff --git a/test/assets/workbook_update_connections_no_auth.xml b/test/assets/workbook_update_connections_no_auth.xml new file mode 100644 index 000000000..21860fa06 --- /dev/null +++ b/test/assets/workbook_update_connections_no_auth.xml @@ -0,0 +1,21 @@ + + + + + + + diff --git a/test/test_datasource.py b/test/test_datasource.py index 56eb11ab7..f776b1c08 100644 --- a/test/test_datasource.py +++ b/test/test_datasource.py @@ -35,6 +35,7 @@ UPDATE_HYPER_DATA_XML = TEST_ASSET_DIR / "datasource_data_update.xml" UPDATE_CONNECTION_XML = TEST_ASSET_DIR / "datasource_connection_update.xml" UPDATE_CONNECTIONS_XML = TEST_ASSET_DIR / "datasource_connections_update.xml" +UPDATE_CONNECTIONS_NO_AUTH_XML = TEST_ASSET_DIR / "datasource_connections_update_no_auth.xml" @pytest.fixture(scope="function") diff --git a/test/test_workbook.py b/test/test_workbook.py index b210e8402..57474548a 100644 --- a/test/test_workbook.py +++ b/test/test_workbook.py @@ -39,6 +39,7 @@ UPDATE_XML = TEST_ASSET_DIR / "workbook_update.xml" UPDATE_PERMISSIONS = TEST_ASSET_DIR / "workbook_update_permissions.xml" UPDATE_CONNECTIONS_XML = TEST_ASSET_DIR / "workbook_update_connections.xml" +UPDATE_CONNECTIONS_NO_AUTH_XML = TEST_ASSET_DIR / "workbook_update_connections_no_auth.xml" @pytest.fixture(scope="function") From 54fada7bfc3c93281175ca7e93837051589bcce8 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Surya Chavatapalli Date: Tue, 5 May 2026 14:34:26 -0500 Subject: [PATCH 2/2] Add tests for optional authentication_type and fix argparse defaults - Remove unnecessary default=None from argparse arguments (implicit for optional args) - Add test_update_connections_without_auth_type for datasources - Add test_update_workbook_connections_without_auth_type for workbooks - Tests verify that auth_type is preserved from server response when not specified These tests ensure the optional authentication_type parameter works correctly and that existing authentication types are preserved when updating only credentials without changing auth type. Co-Authored-By: Claude Sonnet 4.5 --- samples/update_connections_auth.py | 6 ++--- test/test_datasource.py | 38 ++++++++++++++++++++++++++++++ test/test_workbook.py | 36 ++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/samples/update_connections_auth.py b/samples/update_connections_auth.py index 53f1eec81..f99a866ef 100644 --- a/samples/update_connections_auth.py +++ b/samples/update_connections_auth.py @@ -22,9 +22,9 @@ def main(): # Resource-specific parser.add_argument("resource_type", choices=["workbook", "datasource"]) parser.add_argument("resource_id") - parser.add_argument("--datasource_username", default=None, help="Datasource username (optional)") - parser.add_argument("--authentication_type", default=None, help="Authentication type (optional)") - parser.add_argument("--datasource_password", default=None, help="Datasource password (optional)") + parser.add_argument("--datasource_username", help="Datasource username (optional)") + parser.add_argument("--authentication_type", help="Authentication type (optional)") + parser.add_argument("--datasource_password", help="Datasource password (optional)") parser.add_argument( "--embed_password", default="true", choices=["true", "false"], help="Embed password (default: true)" ) diff --git a/test/test_datasource.py b/test/test_datasource.py index f776b1c08..a0890f3a5 100644 --- a/test/test_datasource.py +++ b/test/test_datasource.py @@ -277,6 +277,44 @@ def test_update_connections(server) -> None: assert "auth-keypair" == connection_items[0].auth_type +def test_update_connections_without_auth_type(server) -> None: + """Test that update_connections works when authentication_type is not provided.""" + populate_xml = POPULATE_CONNECTIONS_XML.read_text() + response_xml = UPDATE_CONNECTIONS_NO_AUTH_XML.read_text() + + with requests_mock.Mocker() as m: + + datasource_id = "9dbd2263-16b5-46e1-9c43-a76bb8ab65fb" + connection_luids = ["be786ae0-d2bf-4a4b-9b34-e2de8d2d4488", "a1b2c3d4-e5f6-7a8b-9c0d-123456789abc"] + + datasource = TSC.DatasourceItem(datasource_id) + datasource._id = "9dbd2263-16b5-46e1-9c43-a76bb8ab65fb" + datasource.owner_id = "dd2239f6-ddf1-4107-981a-4cf94e415794" + server.version = "3.26" + + m.get( + "http://test/api/3.26/sites/dad65087-b08b-4603-af4e-2887b8aafc67/datasources/9dbd2263-16b5-46e1-9c43-a76bb8ab65fb/connections", + text=populate_xml, + ) + m.put( + "http://test/api/3.26/sites/dad65087-b08b-4603-af4e-2887b8aafc67/datasources/9dbd2263-16b5-46e1-9c43-a76bb8ab65fb/connections", + text=response_xml, + ) + + # Update connections without specifying authentication_type + connection_items = server.datasources.update_connections( + datasource_item=datasource, + connection_luids=connection_luids, + username="user1", + embed_password=True, + ) + updated_ids = [conn.id for conn in connection_items] + + assert updated_ids == connection_luids + # Verify that the auth type from the response is preserved (UsernamePassword) + assert connection_items[0].auth_type == "UsernamePassword" + + def test_populate_permissions(server) -> None: response_xml = POPULATE_PERMISSIONS_XML.read_text() with requests_mock.mock() as m: diff --git a/test/test_workbook.py b/test/test_workbook.py index 57474548a..c5c4f6662 100644 --- a/test/test_workbook.py +++ b/test/test_workbook.py @@ -1048,6 +1048,42 @@ def test_update_workbook_connections(server: TSC.Server) -> None: assert "AD Service Principal" == connection_items[0].auth_type +def test_update_workbook_connections_without_auth_type(server: TSC.Server) -> None: + """Test that update_connections works when authentication_type is not provided.""" + populate_xml = POPULATE_CONNECTIONS_XML.read_text() + response_xml = UPDATE_CONNECTIONS_NO_AUTH_XML.read_text() + + with requests_mock.Mocker() as m: + workbook_id = "1a2b3c4d-5e6f-7a8b-9c0d-112233445566" + connection_luids = ["abc12345-def6-7890-gh12-ijklmnopqrst", "1234abcd-5678-efgh-ijkl-0987654321mn"] + + workbook = TSC.WorkbookItem(workbook_id) + workbook._id = workbook_id + server.version = "3.26" + url = f"{server.baseurl}/{workbook_id}/connections" + m.get( + "http://test/api/3.26/sites/dad65087-b08b-4603-af4e-2887b8aafc67/workbooks/1a2b3c4d-5e6f-7a8b-9c0d-112233445566/connections", + text=populate_xml, + ) + m.put( + "http://test/api/3.26/sites/dad65087-b08b-4603-af4e-2887b8aafc67/workbooks/1a2b3c4d-5e6f-7a8b-9c0d-112233445566/connections", + text=response_xml, + ) + + # Update connections without specifying authentication_type + connection_items = server.workbooks.update_connections( + workbook_item=workbook, + connection_luids=connection_luids, + username="user1", + embed_password=True, + ) + updated_ids = [conn.id for conn in connection_items] + + assert updated_ids == connection_luids + # Verify that the auth type from the response is preserved (UsernamePassword) + assert connection_items[0].auth_type == "UsernamePassword" + + def test_get_workbook_all_fields(server: TSC.Server) -> None: server.version = "3.21" baseurl = server.workbooks.baseurl