-
Notifications
You must be signed in to change notification settings - Fork 14
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
Deserialize EntryHeader parameters #68
Deserialize EntryHeader parameters #68
Conversation
self.data_type = EntryHeaderDataType(params.get(HEADER_DATA_TYPE_KEY)) | ||
del params[HEADER_DATA_TYPE_KEY] | ||
else: | ||
self.data_type = None |
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.
It could be an argument to put this as EntryHeaderDataType.STRING
since it is the specified fallback in the absence of the "dataType" field. I put it to None
to be transparent about what the server returns.
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.
That's ok, I think. Calling methods can default to String
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.
When fetching a data tree I get the following exception which I don't get in master:
Traceback (most recent call last):
File "/home/user/git/tsp-python-client/./tsp_cli_client", line 364, in <module>
__get_tree(options.uuid, options.get_tree, "DATA_TREE")
File "/home/user/git/tsp-python-client/./tsp_cli_client", line 136, in __get_tree
tree_model.print()
File "/home/user/git/tsp-python-client/tree_model.py", line 57, in print
headers.append(header.name["name"])
TypeError: string indices must be integers
self.data_type = EntryHeaderDataType(params.get(HEADER_DATA_TYPE_KEY)) | ||
del params[HEADER_DATA_TYPE_KEY] | ||
else: | ||
self.data_type = None |
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.
That's ok, I think. Calling methods can default to String
The EntryHeader would set the map of all its parameters as its name member, which semantically does not make sense and requires a user to parse the name member itself to find the actual name of the header. Deserialize the EntryHeader according to the TSP to set members "name", "tooltip" and "data_type".
b7c8acf
to
49af4da
Compare
It should be resolved now. I could print a data-tree successfully after the last update. Nothing else seems to rely on |
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.
Looks good to me. Thanks for the contribution.
The EntryHeader would set the map of all its parameters as its "name" member, which semantically does not make sense and requires a user to parse the name member itself to find the actual name (or other fields) of the header.
Deserialize the EntryHeader according to the TSP to set members "name", "tooltip" and "data_type".
The tests pass for me locally apart from these which also fails on master:
Sidenote: I kept the naming consistent with the existing code, but I am not sure "EntryHeader" describes the class properly. The class "EntryHeader" does not contain a header for a single "Entry", it holds headers for all columns of an "Entry". I believe the naming in the openapi specification "TreeColumnHeader" describes it better. Perhaps here in Python it could be "ColumnHeader" or "EntryColumnHeader"? I could toss that into a separate PR if you agree.
Resolves #33