Skip to content

Commit

Permalink
[FEA] Remove URLs from pydantic error messages (#560)
Browse files Browse the repository at this point in the history
* replaced IllegalArgumentError with PydanticCustomError for improved error message

Signed-off-by: cindyyuanjiang <[email protected]>

* refactored pydantic err message

Signed-off-by: cindyyuanjiang <[email protected]>

* merge conflict

Signed-off-by: cindyyuanjiang <[email protected]>

* fixed incorrect imports

Signed-off-by: cindyyuanjiang <[email protected]>

---------

Signed-off-by: cindyyuanjiang <[email protected]>
  • Loading branch information
cindyyuanjiang authored Sep 20, 2023
1 parent 355f6d2 commit 5c3da28
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 18 deletions.
31 changes: 17 additions & 14 deletions user_tools/src/spark_rapids_tools/cmdli/argprocessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
from pydantic_core import PydanticCustomError

from spark_rapids_tools.cloud import ClientCluster
from spark_rapids_tools.exceptions import IllegalArgumentError
from spark_rapids_tools.utils import AbstractPropContainer, is_http_file
from spark_rapids_pytools.cloud_api.sp_types import DeployMode
from spark_rapids_pytools.common.utilities import ToolLogging
Expand Down Expand Up @@ -115,7 +114,7 @@ def create_tool_args(cls, tool_name: str, *args: Any, **kwargs: Any) -> Optional
impl_class = impl_entry.validator_class
new_obj = impl_class(*args, **kwargs)
return new_obj.build_tools_args()
except (ValidationError, IllegalArgumentError) as e:
except (ValidationError, PydanticCustomError) as e:
impl_class.logger.error('Validation err: %s\n', e)
dump_tool_usage(impl_class.tool_name)
return None
Expand All @@ -126,8 +125,9 @@ def get_eventlogs(self) -> Optional[str]:
return None

def raise_validation_exception(self, validation_err: str):
raise IllegalArgumentError(
f'Invalid arguments: {validation_err}')
raise PydanticCustomError(
'invalid_argument',
f'{validation_err}\n Error:')

def determine_cluster_arg_type(self) -> ArgValueCase:
# self.cluster is provided. then we need to verify that the expected files are there
Expand All @@ -137,13 +137,14 @@ def determine_cluster_arg_type(self) -> ArgValueCase:
# the file cannot be a http_url
if is_http_file(self.cluster):
# we do not accept http://urls
raise IllegalArgumentError(
f'Cluster properties cannot be a web URL path: {self.cluster}')
raise PydanticCustomError(
'invalid_argument',
f'Cluster properties cannot be a web URL path: {self.cluster}\n Error:')
cluster_case = ArgValueCase.VALUE_B
else:
raise PydanticCustomError(
'file_path',
'Cluster property file is not in valid format {.json, .yaml, or .yml}')
'Cluster property file is not in valid format {.json, .yaml, or .yml}\n Error:')
else:
cluster_case = ArgValueCase.VALUE_A
return cluster_case
Expand All @@ -167,8 +168,9 @@ def detect_platform_from_eventlogs_prefix(self):

def validate_onprem_with_cluster_name(self):
if self.platform == CspEnv.ONPREM:
raise IllegalArgumentError(
f'Invalid arguments: Cannot run cluster by name with platform [{CspEnv.ONPREM}]')
raise PydanticCustomError(
'invalid_argument',
f'Cannot run cluster by name with platform [{CspEnv.ONPREM}]\n Error:')

def init_extra_arg_cases(self) -> list:
return []
Expand Down Expand Up @@ -229,8 +231,9 @@ def post_platform_assignment_validation(self, assigned_platform):
if self.argv_cases[1] == ArgValueCase.VALUE_A:
if assigned_platform == CspEnv.ONPREM:
# it is not allowed to run cluster_by_name on an OnPrem platform
raise IllegalArgumentError(
f'Invalid arguments: Cannot run cluster by name with platform [{CspEnv.ONPREM}]')
raise PydanticCustomError(
'invalid_argument',
f'Cannot run cluster by name with platform [{CspEnv.ONPREM}]\n Error:')


@dataclass
Expand Down Expand Up @@ -356,10 +359,10 @@ def build_tools_args(self) -> dict:
self.p_args['toolArgs']['targetPlatform'] = None
else:
if not self.p_args['toolArgs']['targetPlatform'] in equivalent_pricing_list:
raise IllegalArgumentError(
'Invalid arguments: '
raise PydanticCustomError(
'invalid_argument',
f'The platform [{self.p_args["toolArgs"]["targetPlatform"]}] is currently '
f'not supported to calculate savings from [{runtime_platform}] cluster')
f'not supported to calculate savings from [{runtime_platform}] cluster\n Error:')
else:
# target platform is not set, then we disable cost savings if the runtime platform if
# onprem
Expand Down
4 changes: 0 additions & 4 deletions user_tools/src/spark_rapids_tools/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,3 @@ def __init__(self, msg: str, pydantic_err: Optional[ValidationError] = None):
content.append(str.join('. ', single_err))
self.message = str.join('\n', content)
super().__init__(self.message)


class IllegalArgumentError(CspPathException, ValueError):
pass

0 comments on commit 5c3da28

Please sign in to comment.