Improve handling of client dataset creation attempt without domain #537
Conversation
Updating client logic to respond with more informative messages indicating the problem, rather than simply throwing an error, if an attempt is made to create a dataset without providing a valid domain.
robertbartel
force-pushed
the
f/client_domain_fix
branch
from
b3294bc to
375e469
Compare
|
FYI, @aaraney and @hellkite500, I've resolved the previous test failures by merging in recent upstream stuff. |
There was a problem hiding this comment.
Thanks, @robertbartel! Just one comment. Should be good to go after that.
| @@ -64,6 +64,62 @@ def __init__(self, client_config: ClientConfig, bypass_request_service: bool = F | |||
|
|
|||
| self._auth_client: AuthClient = AuthClient(transport_client=self._get_transport_client()) | |||
|
|
|||
| def _extract_dataset_domain(self, **kwargs) -> DataDomain: | |||
There was a problem hiding this comment.
Thoughts on making this a staticmethod just to make it clear that it does not modify the state of the instance, but makes sense to have it here for cohesion reasons?
There was a problem hiding this comment.
Hmm, it's an interesting thought exercise ... I didn't really intend for things to make it possible to call this outside an instance; I just wanted to tidy up the code a bit. On the other hand, aside from implying no state changes, it's probably slightly more performant.
Rather than fall all the way down the rabbit hole, I went ahead and made the change.
|
Merge at will |
Improve the client's behavior when it attempts to create a dataset but no data domain is defined/provided. Previously, an error was raised. Now, a response object will be returned by the relevant function to the client and displayed, indicating that the attempt failed and giving some information on why. E.g., attempting something like this:
will now yield something like this:
{ "message" : "Could not inflate keyword params to object due to RuntimeError - Cannot create DataDomain without at least one finite continuous or discrete restriction", "reason" : "No Dataset Domain Provided", "success" : false }This relates to #531, although strictly speaking this was not a bug preventing valid dataset creation (only an obscuring of an invalid attempt to create one).