Skip to content

Commit fb6712f

Browse files
authored
MPT-17825: resolve WPS430 violations (#161)
🤖 **Codex-generated PR** — Please review carefully. ## Summary - resolve `WPS430` violations by removing nested helper functions - move account/product table-format helpers to dedicated modules - keep API error decorators behavior while removing nested wrappers - remove `WPS430` from flake8 per-file ignores ## Validation - `make check` - `make check-all` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> Closes [MPT-17825](https://softwareone.atlassian.net/browse/MPT-17825) ### Release Notes - Resolved WPS430 violations by removing nested helper functions and nested wrappers - Moved account table formatting helpers into cli/core/accounts/table_formatters.py: wrap_account_type(), wrap_active(), wrap_token() - Moved product table formatting helpers into cli/core/products/table_formatters.py: wrap_product_status(), wrap_vendor() - Added cli/core/error_wrappers.py with HttpErrorWrapper and ApiErrorWrapper to centralize API/HTTP error normalization while preserving decorator behavior - Simplified cli/core/errors.py to delegate error handling to the new wrapper classes - Removed per-file flake8 WPS430 ignores now that violations are fixed - Added unit tests for table formatters and error wrappers; validated changes with make check and make check-all <!-- end of auto-generated comment: release notes by coderabbit.ai --> [MPT-17825]: https://softwareone.atlassian.net/browse/MPT-17825?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
2 parents 45593ee + c091a03 commit fb6712f

File tree

11 files changed

+324
-110
lines changed

11 files changed

+324
-110
lines changed

cli/core/accounts/app.py

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
write_accounts,
1414
)
1515
from cli.core.accounts.models import Account
16+
from cli.core.accounts.table_formatters import wrap_account_type, wrap_active, wrap_token
1617
from cli.core.accounts.url_parser import protocol_and_host
1718
from cli.core.console import console
1819
from cli.core.errors import (
@@ -26,9 +27,6 @@
2627

2728
app = typer.Typer()
2829

29-
NEW_TOKEN_MASK_PREFIX_LENGTH = 22
30-
TOKEN_MASK_PREFIX_LENGTH = 4
31-
3230

3331
@app.command(name="add")
3432
def add_account(
@@ -184,45 +182,15 @@ def _account_table(title: str) -> Table:
184182
return table
185183

186184

187-
def _list_accounts(table: Table, accounts: list[Account], *, wrap_secret: bool = True) -> Table: # noqa: C901
188-
def _wrap_account_type(account_type: str) -> str: # pragma: no cover
189-
match account_type:
190-
case "Vendor":
191-
return f"[cyan]{account_type}"
192-
case "Operations":
193-
return f"[white]{account_type}"
194-
case "Client":
195-
return f"[magenta]{account_type}"
196-
case _:
197-
return account_type
198-
199-
def _wrap_active(*, is_active: bool) -> str: # pragma: no cover
200-
if is_active:
201-
return "[red bold]\u2714"
202-
return ""
203-
204-
def _wrap_token(account: Account, *, to_wrap_secret: bool) -> str:
205-
is_new_token = "idt:TKN-" in account.token
206-
207-
token = account.token if is_new_token else f"{account.token_id}:{account.token}"
208-
209-
if to_wrap_secret:
210-
visible_token_prefix = token[:TOKEN_MASK_PREFIX_LENGTH]
211-
if is_new_token:
212-
token = f"{token[:NEW_TOKEN_MASK_PREFIX_LENGTH]}*****{visible_token_prefix}"
213-
else:
214-
token = f"{token[:TOKEN_MASK_PREFIX_LENGTH]}*****{visible_token_prefix}"
215-
216-
return token
217-
185+
def _list_accounts(table: Table, accounts: list[Account], *, wrap_secret: bool = True) -> Table:
218186
for account in accounts:
219187
table.add_row(
220188
account.id,
221189
account.name,
222-
_wrap_account_type(account.type),
223-
_wrap_token(account, to_wrap_secret=wrap_secret),
190+
wrap_account_type(account.type),
191+
wrap_token(account, to_wrap_secret=wrap_secret),
224192
account.environment,
225-
_wrap_active(is_active=account.is_active),
193+
wrap_active(is_active=account.is_active),
226194
)
227195

228196
return table
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
from cli.core.accounts.models import Account
2+
3+
NEW_TOKEN_MASK_PREFIX_LENGTH = 22
4+
TOKEN_MASK_PREFIX_LENGTH = 4
5+
6+
7+
def wrap_account_type(account_type: str) -> str:
8+
"""Apply rich color formatting for account type."""
9+
match account_type:
10+
case "Vendor":
11+
return f"[cyan]{account_type}"
12+
case "Operations":
13+
return f"[white]{account_type}"
14+
case "Client":
15+
return f"[magenta]{account_type}"
16+
case _:
17+
return account_type
18+
19+
20+
def wrap_active(*, is_active: bool) -> str:
21+
"""Return a marker for active accounts."""
22+
return "[red bold]\u2714" if is_active else ""
23+
24+
25+
def wrap_token(account: Account, *, to_wrap_secret: bool) -> str:
26+
"""Format token display with optional masking."""
27+
is_new_token = "idt:TKN-" in account.token
28+
token = account.token if is_new_token else f"{account.token_id}:{account.token}"
29+
30+
if to_wrap_secret:
31+
visible_token_prefix = token[:TOKEN_MASK_PREFIX_LENGTH]
32+
if is_new_token:
33+
token = f"{token[:NEW_TOKEN_MASK_PREFIX_LENGTH]}*****{visible_token_prefix}"
34+
else:
35+
token = f"{token[:TOKEN_MASK_PREFIX_LENGTH]}*****{visible_token_prefix}"
36+
37+
return token

cli/core/error_wrappers.py

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
from collections.abc import Callable
2+
from functools import partial, update_wrapper
3+
from http import HTTPStatus
4+
from typing import Any, ParamSpec, TypeVar
5+
6+
from mpt_api_client.exceptions import MPTHttpError as APIException
7+
from requests import RequestException, Response
8+
9+
ErrorFactory = Callable[[str, str], Exception]
10+
CallableParams = ParamSpec("CallableParams")
11+
RetType = TypeVar("RetType")
12+
13+
14+
def _parse_bad_request_message(response: Response) -> str:
15+
try:
16+
response_body = response.json()
17+
except ValueError:
18+
return response.text
19+
20+
response_errors = response_body.get("errors", {}) if isinstance(response_body, dict) else {}
21+
if not isinstance(response_errors, dict) or not response_errors:
22+
return response.text
23+
24+
return "\n".join(
25+
(
26+
f"{field}: {error_details[0]}"
27+
if isinstance(error_details, (list, tuple)) and error_details
28+
else f"{field}: {error_details}"
29+
)
30+
for field, error_details in response_errors.items()
31+
)
32+
33+
34+
class HttpErrorWrapper[**CallableParams, RetType]:
35+
"""Wrap callables and normalize RequestException to domain error."""
36+
37+
def __init__(
38+
self,
39+
func: Callable[CallableParams, RetType],
40+
error_factory: ErrorFactory,
41+
):
42+
self._func = func
43+
self._error_factory = error_factory
44+
update_wrapper(self, func)
45+
46+
def __call__(
47+
self,
48+
*args: CallableParams.args,
49+
**kwargs: CallableParams.kwargs,
50+
) -> RetType:
51+
"""Execute wrapped callable and map RequestException to domain error."""
52+
try:
53+
return self._func(*args, **kwargs)
54+
except RequestException as error:
55+
if error.response is None:
56+
msg = "No response"
57+
raise self._error_factory(str(error), msg) from error
58+
59+
if error.response.status_code != HTTPStatus.BAD_REQUEST:
60+
msg = error.response.text
61+
raise self._error_factory(str(error), msg) from error
62+
63+
msg = _parse_bad_request_message(error.response)
64+
raise self._error_factory(str(error), msg) from error
65+
66+
def __get__(self, instance: Any, owner: type[Any] | None = None) -> Any:
67+
if instance is None:
68+
return self
69+
70+
return partial(self.__call__, instance)
71+
72+
73+
class ApiErrorWrapper[**CallableParams, RetType]:
74+
"""Wrap callables and normalize APIException to domain error."""
75+
76+
def __init__(
77+
self,
78+
func: Callable[CallableParams, RetType],
79+
error_factory: ErrorFactory,
80+
):
81+
self._func = func
82+
self._error_factory = error_factory
83+
update_wrapper(self, func)
84+
85+
def __call__(
86+
self,
87+
*args: CallableParams.args,
88+
**kwargs: CallableParams.kwargs,
89+
) -> RetType:
90+
"""Execute wrapped callable and map APIException to domain error."""
91+
try:
92+
return self._func(*args, **kwargs)
93+
except APIException as error:
94+
raise self._error_factory(str(error), error.body) from error
95+
96+
def __get__(self, instance: Any, owner: type[Any] | None = None) -> Any:
97+
if instance is None:
98+
return self
99+
100+
return partial(self.__call__, instance)

cli/core/errors.py

Lines changed: 4 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
from collections.abc import Callable
2-
from functools import wraps
3-
from http import HTTPStatus
4-
from typing import ParamSpec, TypeVar
2+
from typing import ParamSpec, TypeVar, cast
53

6-
from mpt_api_client.exceptions import MPTHttpError as APIException
7-
from requests import RequestException, Response
4+
from cli.core.error_wrappers import ApiErrorWrapper, HttpErrorWrapper
85

96
CallableParams = ParamSpec("CallableParams")
107
RetType = TypeVar("RetType")
@@ -25,26 +22,6 @@ def __str__(self) -> str:
2522
return f"{self._request_msg} with response body {self._response_body}"
2623

2724

28-
def _parse_bad_request_message(response: Response) -> str:
29-
try:
30-
response_body = response.json()
31-
except ValueError:
32-
return str(response.content)
33-
34-
response_errors = response_body.get("errors", {}) if isinstance(response_body, dict) else {}
35-
if not isinstance(response_errors, dict) or not response_errors:
36-
return str(response.content)
37-
38-
return "\n".join(
39-
(
40-
f"{field}: {error_details[0]}"
41-
if isinstance(error_details, (list, tuple)) and error_details
42-
else f"{field}: {error_details}"
43-
)
44-
for field, error_details in response_errors.items()
45-
)
46-
47-
4825
def wrap_http_error[**CallableParams, RetType](
4926
func: Callable[CallableParams, RetType],
5027
) -> Callable[CallableParams, RetType]:
@@ -57,39 +34,14 @@ def wrap_http_error[**CallableParams, RetType](
5734
The wrapped function that raises MPTAPIError on HTTP errors.
5835
5936
"""
60-
61-
@wraps(func)
62-
def _wrapper(*args: CallableParams.args, **kwargs: CallableParams.kwargs) -> RetType:
63-
try:
64-
return func(*args, **kwargs)
65-
except RequestException as error:
66-
if error.response is None:
67-
msg = "No response"
68-
raise MPTAPIError(str(error), msg) from error
69-
70-
if error.response.status_code != HTTPStatus.BAD_REQUEST:
71-
msg = str(error.response.content)
72-
raise MPTAPIError(str(error), msg) from error
73-
74-
msg = _parse_bad_request_message(error.response)
75-
raise MPTAPIError(str(error), msg) from error
76-
77-
return _wrapper
37+
return cast(Callable[CallableParams, RetType], HttpErrorWrapper(func, MPTAPIError))
7838

7939

8040
def wrap_mpt_api_error[**CallableParams, RetType](
8141
func: Callable[CallableParams, RetType],
8242
) -> Callable[CallableParams, RetType]:
8343
"""Decorator to wrap MPT API functions and handle APIException."""
84-
85-
@wraps(func)
86-
def _wrapper(*args: CallableParams.args, **kwargs: CallableParams.kwargs) -> RetType:
87-
try:
88-
return func(*args, **kwargs)
89-
except APIException as error:
90-
raise MPTAPIError(str(error), error.body) from error
91-
92-
return _wrapper
44+
return cast(Callable[CallableParams, RetType], ApiErrorWrapper(func, MPTAPIError))
9345

9446

9547
class AccountNotFoundError(CLIError):

cli/core/products/app.py

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
from cli.core.accounts.containers import AccountContainer
77
from cli.core.console import console
88
from cli.core.mpt.flows import get_products
9-
from cli.core.mpt.models import Account as MPTAccount
109
from cli.core.mpt.models import Product as MPTProduct
1110
from cli.core.mpt.mpt_client import create_api_mpt_client_from_account
1211
from cli.core.products.containers import ProductContainer
12+
from cli.core.products.table_formatters import wrap_product_status, wrap_vendor
1313
from rich import box
1414
from rich.status import Status
1515
from rich.table import Table
@@ -336,29 +336,13 @@ def _products_table(title: str) -> Table:
336336

337337

338338
# TODO: move to to_table()
339-
def _list_products(table: Table, products: list[MPTProduct]) -> Table: # noqa: C901
340-
def _wrap_product_status(status: str) -> str: # pragma: no cover
341-
match status:
342-
case "Draft":
343-
return f"[white]{status}"
344-
case "Pending":
345-
return f"[blue]{status}"
346-
case "Published":
347-
return f"[green bold]{status}"
348-
case "Unpublished":
349-
return f"[red]{status}"
350-
case _:
351-
return status
352-
353-
def _wrap_vendor(vendor: MPTAccount) -> str: # pragma: no cover
354-
return f"{vendor.id} ({vendor.name})"
355-
339+
def _list_products(table: Table, products: list[MPTProduct]) -> Table:
356340
for product in products:
357341
table.add_row(
358342
product.id,
359343
product.name,
360-
_wrap_product_status(product.status),
361-
_wrap_vendor(product.vendor),
344+
wrap_product_status(product.status),
345+
wrap_vendor(product.vendor),
362346
)
363347

364348
return table
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from cli.core.mpt.models import Account as MPTAccount
2+
3+
4+
def wrap_product_status(status: str) -> str:
5+
"""Apply rich color formatting for product status."""
6+
match status:
7+
case "Draft":
8+
return f"[white]{status}"
9+
case "Pending":
10+
return f"[blue]{status}"
11+
case "Published":
12+
return f"[green bold]{status}"
13+
case "Unpublished":
14+
return f"[red]{status}"
15+
case _:
16+
return status
17+
18+
19+
def wrap_vendor(vendor: MPTAccount) -> str:
20+
"""Format vendor account details for table output."""
21+
return f"{vendor.id} ({vendor.name})"

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ select = ["AAA", "E999", "WPS"]
121121
show-source = true
122122
statistics = false
123123
per-file-ignores = [
124-
"*.py:WPS204,WPS210,WPS211,WPS213,WPS214,WPS218,WPS221,WPS229,WPS231,WPS232,WPS234,WPS235,WPS237,WPS322,WPS327,WPS332,WPS335,WPS336,WPS338,WPS339,WPS342,WPS347,WPS349,WPS400,WPS402,WPS404,WPS407,WPS410,WPS412,WPS420,WPS430,WPS432,WPS458,WPS471,WPS478",
124+
"*.py:WPS204,WPS210,WPS211,WPS213,WPS214,WPS218,WPS221,WPS229,WPS231,WPS232,WPS234,WPS235,WPS237,WPS322,WPS327,WPS332,WPS335,WPS336,WPS338,WPS339,WPS342,WPS347,WPS349,WPS400,WPS402,WPS404,WPS407,WPS410,WPS412,WPS420,WPS432,WPS458,WPS471,WPS478",
125125
"tests/**:WPS202,WPS203,WPS204,WPS210,WPS211,WPS213,WPS218,WPS221,WPS235,WPS322,WPS335,WPS339,WPS342,WPS432"
126126
]
127127

0 commit comments

Comments
 (0)