Conversation
50f1163 to
304622b
Compare
csadorf
left a comment
There was a problem hiding this comment.
Thank you for the quick fix, however it missing the data models for the return types as pointed out inline.
csadorf
left a comment
There was a problem hiding this comment.
The functions are still missing the appropriate response models. I am very sorry that my previous comment was in so far easy to misunderstand.
As an example, the correct response model for the create_dataset function is DatasetCreateResponse. This can be inferred from the standard-app-api definition. In other cases it might be a bit less obvious, e.g., for the get_dataset_metadata function I would expect to simply receive a plain dict.
The standard-app-api implements the API specification and also serves as a prototype for the actual server implementation. The server expects Response types, because it is a web server. However, the SDK is to be used by application developers. They do not need to worry about the HTTP responses anymore, here we should work with Python types as appropriate. So, coming back to the original example, getDatasetMetadata function is to be used to actually obtain the dataset metadata (as a dict), not to generate a server response.
6de16e6 to
93800e6
Compare
| metadata: dict = None, | ||
| collection_name: object_storage.CollectionName = None, | ||
| ) -> str: | ||
| ) -> Union[Dict, str]: |
There was a problem hiding this comment.
Is there still a path where this function would return str?
(Same question for all other similar occurrences.)
Per discussion: #43 (review)
Changing from string response to json