-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Configure TestClient
to use internal error handling mechanism
#115
Conversation
@@ -16,6 +19,9 @@ | |||
|
|||
|
|||
class BaseModel(RootBaseModel, mixins.HasCreationInfo): | |||
# NOTE: only subclasses storing data actually define this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defining the type here so that I can access BaseModel.DataInvalid
in the utils.validate_data()
function. This might not be necessary though (we could also just raise OptimizationDataValidationError
s straight from ixmp4.core.exceptions
there) and it might be confusing since not all subclasses (like Column
) actually define this error (since they don't all store and validate data).
So please confirm this solution is fine or please suggest an alternative.
@validates("data") | ||
def validate_data(self, key, data: dict[str, Any]): | ||
return utils.validate_data( | ||
key=key, | ||
host=self, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing the whole instance at the moment to access both .DataInvalid
and __str__
, but if we change the way we raise errros in utils.validate_data()
, we might also be able to just pass __str__
.
As noted by @meksor in his review of #97 , raising
ValueError
s shouldn't work when working with the REST API. This PR disables this behaviour, so these PRs will need a little reworking and if we missed any such errors before, the checks on this PR will fail and we need some reworking here, too.