From 992f89eb1be1429c90b1354ddfec981a9075714c Mon Sep 17 00:00:00 2001 From: Alex Moraru Date: Mon, 12 Jan 2026 08:30:19 +0000 Subject: [PATCH 1/6] Small test fixes --- src/fabric_cli/commands/fs/fab_fs.py | 44 +++++++----- src/fabric_cli/core/fab_exceptions.py | 99 +++++++++++++++------------ tests/test_commands/test_import.py | 9 +++ 3 files changed, 94 insertions(+), 58 deletions(-) diff --git a/src/fabric_cli/commands/fs/fab_fs.py b/src/fabric_cli/commands/fs/fab_fs.py index fb707302..92181e64 100644 --- a/src/fabric_cli/commands/fs/fab_fs.py +++ b/src/fabric_cli/commands/fs/fab_fs.py @@ -301,22 +301,34 @@ def _search_capacity_id(capacity_name: str) -> str | None: def _get_capacity_id( capacity_name, subscription_id=None, resource_group_name=None ) -> str | None: - if subscription_id and resource_group_name: - args = Namespace() - args.name = capacity_name - args.subscription_id = subscription_id - args.resource_group_name = resource_group_name - try: - response = capacity_api.get_capacity(args) - except FabricCLIError: - return _search_capacity_id(capacity_name) - else: - if response.status_code == 200: - return json.loads(response.text)["id"] - else: - return _search_capacity_id(capacity_name) - else: - return _search_capacity_id(capacity_name) + import time + + max_retries = 3 + retry_delay = 2 # seconds + + for attempt in range(max_retries): + if subscription_id and resource_group_name: + args = Namespace() + args.name = capacity_name + args.subscription_id = subscription_id + args.resource_group_name = resource_group_name + try: + response = capacity_api.get_capacity(args) + if response.status_code == 200: + return json.loads(response.text)["id"] + except FabricCLIError: + pass # Continue to fallback search + + # Fallback to searching across subscriptions + capacity_id = _search_capacity_id(capacity_name) + if capacity_id: + return capacity_id + + # If not found and we still have retries left, wait and try again + if attempt < max_retries - 1: + time.sleep(retry_delay) + + return None def get_all_az_capacities() -> list: diff --git a/src/fabric_cli/core/fab_exceptions.py b/src/fabric_cli/core/fab_exceptions.py index 1035c037..abf6ab6a 100644 --- a/src/fabric_cli/core/fab_exceptions.py +++ b/src/fabric_cli/core/fab_exceptions.py @@ -9,7 +9,7 @@ class FabricCLIError(Exception): def __init__(self, message, status_code=None): super().__init__(message) - self.message = message.rstrip(".") + self.message = message.rstrip(".") if message else None self.status_code = status_code def __str__(self): @@ -20,7 +20,7 @@ def __str__(self): ) def formatted_message(self, verbose=False): - escaped_text = html.escape(self.message) + escaped_text = html.escape(self.message) if self.message else "" return ( f"[{self.status_code}] {escaped_text}" @@ -63,12 +63,17 @@ def __init__(self, response_text): related_resource (dict): Details about the main related resource, if available. request_id (str): The ID of the request associated with the error. """ - response = json.loads(response_text) - message = response.get("message") - error_code = response.get("errorCode") - - self.more_details: list[dict] = response.get("moreDetails", []) - self.request_id = response.get("requestId") + try: + response = json.loads(response_text) if response_text else {} + message = response.get("message") + error_code = response.get("errorCode") + self.more_details: list[dict] = response.get("moreDetails", []) + self.request_id = response.get("requestId") + except (json.JSONDecodeError, TypeError): + message = "An error occurred while processing the operation" + error_code = "UnknownError" + self.more_details = [] + self.request_id = None super().__init__(message, error_code) @@ -111,27 +116,32 @@ def __init__(self, response_text): code (str): The error code returned by the API. message (str): A descriptive message about the error. """ - response_data = json.loads(response_text) - error_data = response_data.get("error", {}) - code = error_data.get("code") - message = error_data.get("message") - - if message: - message = re.sub(r"\n(?=RequestId:)", "", message) - match = re.search(r"RequestId:(\S+)", message) - if match: - self.request_id = match.group(1) - message = message.replace(match.group(0), "") - else: - self.request_id = None - - message = re.sub(r"\n(?=Time:)", "", message) - match = re.search(r"Time:(\S+)", message) - if match: - self.timestamp = match.group(1) - message = message.replace(match.group(0), "") - else: - self.timestamp = None + try: + response_data = json.loads(response_text) if response_text else {} + error_data = response_data.get("error", {}) + code = error_data.get("code") + message = error_data.get("message") + + self.request_id = None + self.timestamp = None + + if message: + message = re.sub(r"\n(?=RequestId:)", "", message) + match = re.search(r"RequestId:(\S+)", message) + if match: + self.request_id = match.group(1) + message = message.replace(match.group(0), "") + + message = re.sub(r"\n(?=Time:)", "", message) + match = re.search(r"Time:(\S+)", message) + if match: + self.timestamp = match.group(1) + message = message.replace(match.group(0), "") + except (json.JSONDecodeError, TypeError): + message = "An error occurred while processing the operation" + code = "UnknownError" + self.request_id = None + self.timestamp = None super().__init__(message, code) @@ -188,19 +198,24 @@ def __init__(self, response_text): details (list): A list of additional error details, if available. additional_info (list): Additional info at the main error level, if available. """ - response_data = json.loads(response_text) - error_data = response_data.get("error", {}) - code = error_data.get("code") - message = error_data.get("message") - - details: list[dict] = error_data.get("details", []) - - # Extract RootActivityId from the details - self.request_id = None - for detail in details: - if detail.get("code") == "RootActivityId": - self.request_id = detail.get("message") - break + try: + response_data = json.loads(response_text) if response_text else {} + error_data = response_data.get("error", {}) + code = error_data.get("code") + message = error_data.get("message") + + details: list[dict] = error_data.get("details", []) + + # Extract RootActivityId from the details + self.request_id = None + for detail in details: + if detail.get("code") == "RootActivityId": + self.request_id = detail.get("message") + break + except (json.JSONDecodeError, TypeError): + message = "An error occurred while processing the operation" + code = "UnknownError" + self.request_id = None super().__init__(message, code) diff --git a/tests/test_commands/test_import.py b/tests/test_commands/test_import.py index 674a6ea4..d96deca3 100644 --- a/tests/test_commands/test_import.py +++ b/tests/test_commands/test_import.py @@ -4,6 +4,7 @@ import argparse import os import platform +import time from unittest.mock import ANY, patch import pytest @@ -847,6 +848,10 @@ def _import_create_new_item_success( ): # Setup item = item_factory(item_type) + + if item_type == ItemType.MIRRORED_DATABASE: + time.sleep(60) + export(item.full_path, output=os.path.expanduser(str(tmp_path))) # Reset mock @@ -891,6 +896,10 @@ def _import_update_existing_item_success( ): # Setup item = item_factory(item_type) + + if item_type == ItemType.MIRRORED_DATABASE: + time.sleep(60) + export(item.full_path, output=str(tmp_path)) # Reset mock From a8b4b4801c8cecb8d2090beccb7b30469713859d Mon Sep 17 00:00:00 2001 From: Alex Moraru Date: Tue, 20 Jan 2026 13:54:43 +0000 Subject: [PATCH 2/6] Code review optimizations --- src/fabric_cli/commands/fs/fab_fs.py | 44 ++++++++++----------------- src/fabric_cli/core/fab_exceptions.py | 31 ++++++++++--------- tests/test_commands/test_import.py | 2 ++ 3 files changed, 35 insertions(+), 42 deletions(-) diff --git a/src/fabric_cli/commands/fs/fab_fs.py b/src/fabric_cli/commands/fs/fab_fs.py index 92181e64..fb707302 100644 --- a/src/fabric_cli/commands/fs/fab_fs.py +++ b/src/fabric_cli/commands/fs/fab_fs.py @@ -301,34 +301,22 @@ def _search_capacity_id(capacity_name: str) -> str | None: def _get_capacity_id( capacity_name, subscription_id=None, resource_group_name=None ) -> str | None: - import time - - max_retries = 3 - retry_delay = 2 # seconds - - for attempt in range(max_retries): - if subscription_id and resource_group_name: - args = Namespace() - args.name = capacity_name - args.subscription_id = subscription_id - args.resource_group_name = resource_group_name - try: - response = capacity_api.get_capacity(args) - if response.status_code == 200: - return json.loads(response.text)["id"] - except FabricCLIError: - pass # Continue to fallback search - - # Fallback to searching across subscriptions - capacity_id = _search_capacity_id(capacity_name) - if capacity_id: - return capacity_id - - # If not found and we still have retries left, wait and try again - if attempt < max_retries - 1: - time.sleep(retry_delay) - - return None + if subscription_id and resource_group_name: + args = Namespace() + args.name = capacity_name + args.subscription_id = subscription_id + args.resource_group_name = resource_group_name + try: + response = capacity_api.get_capacity(args) + except FabricCLIError: + return _search_capacity_id(capacity_name) + else: + if response.status_code == 200: + return json.loads(response.text)["id"] + else: + return _search_capacity_id(capacity_name) + else: + return _search_capacity_id(capacity_name) def get_all_az_capacities() -> list: diff --git a/src/fabric_cli/core/fab_exceptions.py b/src/fabric_cli/core/fab_exceptions.py index abf6ab6a..e64d1075 100644 --- a/src/fabric_cli/core/fab_exceptions.py +++ b/src/fabric_cli/core/fab_exceptions.py @@ -5,6 +5,10 @@ import json import re +# Constants for common error messages and codes +DEFAULT_ERROR_MESSAGE = "An error occurred while processing the operation" +DEFAULT_ERROR_CODE = "UnknownError" + class FabricCLIError(Exception): def __init__(self, message, status_code=None): @@ -70,8 +74,8 @@ def __init__(self, response_text): self.more_details: list[dict] = response.get("moreDetails", []) self.request_id = response.get("requestId") except (json.JSONDecodeError, TypeError): - message = "An error occurred while processing the operation" - error_code = "UnknownError" + message = DEFAULT_ERROR_MESSAGE + error_code = DEFAULT_ERROR_CODE self.more_details = [] self.request_id = None @@ -116,15 +120,16 @@ def __init__(self, response_text): code (str): The error code returned by the API. message (str): A descriptive message about the error. """ + # Initialize properties before parsing + self.request_id = None + self.timestamp = None + try: response_data = json.loads(response_text) if response_text else {} error_data = response_data.get("error", {}) code = error_data.get("code") message = error_data.get("message") - self.request_id = None - self.timestamp = None - if message: message = re.sub(r"\n(?=RequestId:)", "", message) match = re.search(r"RequestId:(\S+)", message) @@ -138,10 +143,8 @@ def __init__(self, response_text): self.timestamp = match.group(1) message = message.replace(match.group(0), "") except (json.JSONDecodeError, TypeError): - message = "An error occurred while processing the operation" - code = "UnknownError" - self.request_id = None - self.timestamp = None + message = DEFAULT_ERROR_MESSAGE + code = DEFAULT_ERROR_CODE super().__init__(message, code) @@ -198,6 +201,9 @@ def __init__(self, response_text): details (list): A list of additional error details, if available. additional_info (list): Additional info at the main error level, if available. """ + # Initialize properties before parsing + self.request_id = None + try: response_data = json.loads(response_text) if response_text else {} error_data = response_data.get("error", {}) @@ -206,16 +212,13 @@ def __init__(self, response_text): details: list[dict] = error_data.get("details", []) - # Extract RootActivityId from the details - self.request_id = None for detail in details: if detail.get("code") == "RootActivityId": self.request_id = detail.get("message") break except (json.JSONDecodeError, TypeError): - message = "An error occurred while processing the operation" - code = "UnknownError" - self.request_id = None + message = DEFAULT_ERROR_MESSAGE + code = DEFAULT_ERROR_CODE super().__init__(message, code) diff --git a/tests/test_commands/test_import.py b/tests/test_commands/test_import.py index d96deca3..21850a16 100644 --- a/tests/test_commands/test_import.py +++ b/tests/test_commands/test_import.py @@ -849,6 +849,7 @@ def _import_create_new_item_success( # Setup item = item_factory(item_type) + # TODO: delete this line after mirrored db fix the API GAP for Create if item_type == ItemType.MIRRORED_DATABASE: time.sleep(60) @@ -897,6 +898,7 @@ def _import_update_existing_item_success( # Setup item = item_factory(item_type) + # TODO: delete this line after mirrored db fix the API GAP for Create if item_type == ItemType.MIRRORED_DATABASE: time.sleep(60) From ecfe29b5a32a132dbb36f21d4f6a0e57bbb1a00f Mon Sep 17 00:00:00 2001 From: Alex Moraru Date: Wed, 21 Jan 2026 12:21:12 +0000 Subject: [PATCH 3/6] Adds changelog --- .changes/unreleased/fixed-20260121-141955.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/fixed-20260121-141955.yaml diff --git a/.changes/unreleased/fixed-20260121-141955.yaml b/.changes/unreleased/fixed-20260121-141955.yaml new file mode 100644 index 00000000..4c7460d4 --- /dev/null +++ b/.changes/unreleased/fixed-20260121-141955.yaml @@ -0,0 +1,6 @@ +kind: fixed +body: Adds sleep time for tests using mirrored database & null checks +time: 2026-01-21T14:19:55.738181+02:00 +custom: + Author: v-alexmoraru + AuthorLink: https://github.com/v-alexmoraru From f9e8523235c248b4641d6a7bbadd31de36553b62 Mon Sep 17 00:00:00 2001 From: Alex Moraru Date: Mon, 26 Jan 2026 07:08:27 +0000 Subject: [PATCH 4/6] Moves constants to separate file --- .changes/unreleased/fixed-20260121-141955.yaml | 6 ------ src/fabric_cli/core/fab_constant.py | 7 ++++++- src/fabric_cli/core/fab_exceptions.py | 8 +++----- 3 files changed, 9 insertions(+), 12 deletions(-) delete mode 100644 .changes/unreleased/fixed-20260121-141955.yaml diff --git a/.changes/unreleased/fixed-20260121-141955.yaml b/.changes/unreleased/fixed-20260121-141955.yaml deleted file mode 100644 index 4c7460d4..00000000 --- a/.changes/unreleased/fixed-20260121-141955.yaml +++ /dev/null @@ -1,6 +0,0 @@ -kind: fixed -body: Adds sleep time for tests using mirrored database & null checks -time: 2026-01-21T14:19:55.738181+02:00 -custom: - Author: v-alexmoraru - AuthorLink: https://github.com/v-alexmoraru diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index 2e7d23c9..e409fd5d 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -19,7 +19,8 @@ ) API_ENDPOINT_POWER_BI = ( - validate_and_get_env_variable("FAB_API_ENDPOINT_POWER_BI", "api.powerbi.com") + validate_and_get_env_variable( + "FAB_API_ENDPOINT_POWER_BI", "api.powerbi.com") + "/v1.0/myorg" ) @@ -235,6 +236,10 @@ WARNING_WORKSPACE_EMPTY = "Workspace is empty" WARNING_ITEM_EXISTS_IN_PATH = "An item with the same name exists in {0}" +# General errors +DEFAULT_ERROR_MESSAGE = "An error occurred while processing the operation" +DEFAULT_ERROR_CODE = "UnknownError" + # Error codes ERROR_ALREADY_EXISTS = "AlreadyExists" diff --git a/src/fabric_cli/core/fab_exceptions.py b/src/fabric_cli/core/fab_exceptions.py index e64d1075..88491da3 100644 --- a/src/fabric_cli/core/fab_exceptions.py +++ b/src/fabric_cli/core/fab_exceptions.py @@ -5,9 +5,7 @@ import json import re -# Constants for common error messages and codes -DEFAULT_ERROR_MESSAGE = "An error occurred while processing the operation" -DEFAULT_ERROR_CODE = "UnknownError" +from fabric_cli.core.fab_constant import DEFAULT_ERROR_MESSAGE, DEFAULT_ERROR_CODE class FabricCLIError(Exception): @@ -123,7 +121,7 @@ def __init__(self, response_text): # Initialize properties before parsing self.request_id = None self.timestamp = None - + try: response_data = json.loads(response_text) if response_text else {} error_data = response_data.get("error", {}) @@ -203,7 +201,7 @@ def __init__(self, response_text): """ # Initialize properties before parsing self.request_id = None - + try: response_data = json.loads(response_text) if response_text else {} error_data = response_data.get("error", {}) From c3f09fdad327d57b8ffe9f172dbcb8300bffb7b5 Mon Sep 17 00:00:00 2001 From: Alex Moraru Date: Mon, 26 Jan 2026 07:55:19 +0000 Subject: [PATCH 5/6] Moves constants to avoid circular imports --- src/fabric_cli/core/fab_constant.py | 4 ---- src/fabric_cli/core/fab_exceptions.py | 4 +++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index e409fd5d..e9c75365 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -236,10 +236,6 @@ WARNING_WORKSPACE_EMPTY = "Workspace is empty" WARNING_ITEM_EXISTS_IN_PATH = "An item with the same name exists in {0}" -# General errors -DEFAULT_ERROR_MESSAGE = "An error occurred while processing the operation" -DEFAULT_ERROR_CODE = "UnknownError" - # Error codes ERROR_ALREADY_EXISTS = "AlreadyExists" diff --git a/src/fabric_cli/core/fab_exceptions.py b/src/fabric_cli/core/fab_exceptions.py index 88491da3..400a060f 100644 --- a/src/fabric_cli/core/fab_exceptions.py +++ b/src/fabric_cli/core/fab_exceptions.py @@ -5,7 +5,9 @@ import json import re -from fabric_cli.core.fab_constant import DEFAULT_ERROR_MESSAGE, DEFAULT_ERROR_CODE +# Default error constants - avoids circular imports +DEFAULT_ERROR_MESSAGE = "An error occurred while processing the operation" +DEFAULT_ERROR_CODE = "UnknownError" class FabricCLIError(Exception): From 3ac1a9f260088337493499eb53d6f868695d6b24 Mon Sep 17 00:00:00 2001 From: Alex Moraru Date: Tue, 27 Jan 2026 11:36:38 +0000 Subject: [PATCH 6/6] Optimizes exception handling --- src/fabric_cli/core/fab_exceptions.py | 90 +++++++++++++-------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/src/fabric_cli/core/fab_exceptions.py b/src/fabric_cli/core/fab_exceptions.py index 400a060f..7757a150 100644 --- a/src/fabric_cli/core/fab_exceptions.py +++ b/src/fabric_cli/core/fab_exceptions.py @@ -11,11 +11,22 @@ class FabricCLIError(Exception): - def __init__(self, message, status_code=None): + def __init__(self, message=None, status_code=None): + # Use default values if not provided + message = message or DEFAULT_ERROR_MESSAGE + status_code = status_code or DEFAULT_ERROR_CODE + super().__init__(message) self.message = message.rstrip(".") if message else None self.status_code = status_code + @staticmethod + def _parse_json_response(response_text): + try: + return json.loads(response_text) if response_text else {} + except (json.JSONDecodeError, TypeError): + return {} + def __str__(self): return ( f"[{self.status_code}] {self.args[0]}" @@ -67,17 +78,12 @@ def __init__(self, response_text): related_resource (dict): Details about the main related resource, if available. request_id (str): The ID of the request associated with the error. """ - try: - response = json.loads(response_text) if response_text else {} - message = response.get("message") - error_code = response.get("errorCode") - self.more_details: list[dict] = response.get("moreDetails", []) - self.request_id = response.get("requestId") - except (json.JSONDecodeError, TypeError): - message = DEFAULT_ERROR_MESSAGE - error_code = DEFAULT_ERROR_CODE - self.more_details = [] - self.request_id = None + response = self._parse_json_response(response_text) + + message = response.get("message") + error_code = response.get("errorCode") + self.more_details: list[dict] = response.get("moreDetails", []) + self.request_id = response.get("requestId") super().__init__(message, error_code) @@ -124,27 +130,23 @@ def __init__(self, response_text): self.request_id = None self.timestamp = None - try: - response_data = json.loads(response_text) if response_text else {} - error_data = response_data.get("error", {}) - code = error_data.get("code") - message = error_data.get("message") - - if message: - message = re.sub(r"\n(?=RequestId:)", "", message) - match = re.search(r"RequestId:(\S+)", message) - if match: - self.request_id = match.group(1) - message = message.replace(match.group(0), "") - - message = re.sub(r"\n(?=Time:)", "", message) - match = re.search(r"Time:(\S+)", message) - if match: - self.timestamp = match.group(1) - message = message.replace(match.group(0), "") - except (json.JSONDecodeError, TypeError): - message = DEFAULT_ERROR_MESSAGE - code = DEFAULT_ERROR_CODE + response_data = self._parse_json_response(response_text) + error_data = response_data.get("error", {}) + code = error_data.get("code") + message = error_data.get("message") + + if message: + message = re.sub(r"\n(?=RequestId:)", "", message) + match = re.search(r"RequestId:(\S+)", message) + if match: + self.request_id = match.group(1) + message = message.replace(match.group(0), "") + + message = re.sub(r"\n(?=Time:)", "", message) + match = re.search(r"Time:(\S+)", message) + if match: + self.timestamp = match.group(1) + message = message.replace(match.group(0), "") super().__init__(message, code) @@ -204,21 +206,17 @@ def __init__(self, response_text): # Initialize properties before parsing self.request_id = None - try: - response_data = json.loads(response_text) if response_text else {} - error_data = response_data.get("error", {}) - code = error_data.get("code") - message = error_data.get("message") + response_data = self._parse_json_response(response_text) + error_data = response_data.get("error", {}) + code = error_data.get("code") + message = error_data.get("message") - details: list[dict] = error_data.get("details", []) + details: list[dict] = error_data.get("details", []) - for detail in details: - if detail.get("code") == "RootActivityId": - self.request_id = detail.get("message") - break - except (json.JSONDecodeError, TypeError): - message = DEFAULT_ERROR_MESSAGE - code = DEFAULT_ERROR_CODE + for detail in details: + if detail.get("code") == "RootActivityId": + self.request_id = detail.get("message") + break super().__init__(message, code)