Skip to content

Commit a8b4b48

Browse files
committed
Code review optimizations
1 parent 992f89e commit a8b4b48

3 files changed

Lines changed: 35 additions & 42 deletions

File tree

src/fabric_cli/commands/fs/fab_fs.py

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -301,34 +301,22 @@ def _search_capacity_id(capacity_name: str) -> str | None:
301301
def _get_capacity_id(
302302
capacity_name, subscription_id=None, resource_group_name=None
303303
) -> str | None:
304-
import time
305-
306-
max_retries = 3
307-
retry_delay = 2 # seconds
308-
309-
for attempt in range(max_retries):
310-
if subscription_id and resource_group_name:
311-
args = Namespace()
312-
args.name = capacity_name
313-
args.subscription_id = subscription_id
314-
args.resource_group_name = resource_group_name
315-
try:
316-
response = capacity_api.get_capacity(args)
317-
if response.status_code == 200:
318-
return json.loads(response.text)["id"]
319-
except FabricCLIError:
320-
pass # Continue to fallback search
321-
322-
# Fallback to searching across subscriptions
323-
capacity_id = _search_capacity_id(capacity_name)
324-
if capacity_id:
325-
return capacity_id
326-
327-
# If not found and we still have retries left, wait and try again
328-
if attempt < max_retries - 1:
329-
time.sleep(retry_delay)
330-
331-
return None
304+
if subscription_id and resource_group_name:
305+
args = Namespace()
306+
args.name = capacity_name
307+
args.subscription_id = subscription_id
308+
args.resource_group_name = resource_group_name
309+
try:
310+
response = capacity_api.get_capacity(args)
311+
except FabricCLIError:
312+
return _search_capacity_id(capacity_name)
313+
else:
314+
if response.status_code == 200:
315+
return json.loads(response.text)["id"]
316+
else:
317+
return _search_capacity_id(capacity_name)
318+
else:
319+
return _search_capacity_id(capacity_name)
332320

333321

334322
def get_all_az_capacities() -> list:

src/fabric_cli/core/fab_exceptions.py

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
import json
66
import re
77

8+
# Constants for common error messages and codes
9+
DEFAULT_ERROR_MESSAGE = "An error occurred while processing the operation"
10+
DEFAULT_ERROR_CODE = "UnknownError"
11+
812

913
class FabricCLIError(Exception):
1014
def __init__(self, message, status_code=None):
@@ -70,8 +74,8 @@ def __init__(self, response_text):
7074
self.more_details: list[dict] = response.get("moreDetails", [])
7175
self.request_id = response.get("requestId")
7276
except (json.JSONDecodeError, TypeError):
73-
message = "An error occurred while processing the operation"
74-
error_code = "UnknownError"
77+
message = DEFAULT_ERROR_MESSAGE
78+
error_code = DEFAULT_ERROR_CODE
7579
self.more_details = []
7680
self.request_id = None
7781

@@ -116,15 +120,16 @@ def __init__(self, response_text):
116120
code (str): The error code returned by the API.
117121
message (str): A descriptive message about the error.
118122
"""
123+
# Initialize properties before parsing
124+
self.request_id = None
125+
self.timestamp = None
126+
119127
try:
120128
response_data = json.loads(response_text) if response_text else {}
121129
error_data = response_data.get("error", {})
122130
code = error_data.get("code")
123131
message = error_data.get("message")
124132

125-
self.request_id = None
126-
self.timestamp = None
127-
128133
if message:
129134
message = re.sub(r"\n(?=RequestId:)", "", message)
130135
match = re.search(r"RequestId:(\S+)", message)
@@ -138,10 +143,8 @@ def __init__(self, response_text):
138143
self.timestamp = match.group(1)
139144
message = message.replace(match.group(0), "")
140145
except (json.JSONDecodeError, TypeError):
141-
message = "An error occurred while processing the operation"
142-
code = "UnknownError"
143-
self.request_id = None
144-
self.timestamp = None
146+
message = DEFAULT_ERROR_MESSAGE
147+
code = DEFAULT_ERROR_CODE
145148

146149
super().__init__(message, code)
147150

@@ -198,6 +201,9 @@ def __init__(self, response_text):
198201
details (list): A list of additional error details, if available.
199202
additional_info (list): Additional info at the main error level, if available.
200203
"""
204+
# Initialize properties before parsing
205+
self.request_id = None
206+
201207
try:
202208
response_data = json.loads(response_text) if response_text else {}
203209
error_data = response_data.get("error", {})
@@ -206,16 +212,13 @@ def __init__(self, response_text):
206212

207213
details: list[dict] = error_data.get("details", [])
208214

209-
# Extract RootActivityId from the details
210-
self.request_id = None
211215
for detail in details:
212216
if detail.get("code") == "RootActivityId":
213217
self.request_id = detail.get("message")
214218
break
215219
except (json.JSONDecodeError, TypeError):
216-
message = "An error occurred while processing the operation"
217-
code = "UnknownError"
218-
self.request_id = None
220+
message = DEFAULT_ERROR_MESSAGE
221+
code = DEFAULT_ERROR_CODE
219222

220223
super().__init__(message, code)
221224

tests/test_commands/test_import.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,7 @@ def _import_create_new_item_success(
849849
# Setup
850850
item = item_factory(item_type)
851851

852+
# TODO: delete this line after mirrored db fix the API GAP for Create
852853
if item_type == ItemType.MIRRORED_DATABASE:
853854
time.sleep(60)
854855

@@ -897,6 +898,7 @@ def _import_update_existing_item_success(
897898
# Setup
898899
item = item_factory(item_type)
899900

901+
# TODO: delete this line after mirrored db fix the API GAP for Create
900902
if item_type == ItemType.MIRRORED_DATABASE:
901903
time.sleep(60)
902904

0 commit comments

Comments
 (0)