Ds10 #21
Ds10 #21
Conversation
There was a problem hiding this comment.
These changes seem good and follow the libraries implementation elsewhere very well. Great that you included tests.
The only comments I had are very minor and should affect operation at all. There are some indentation and spacing issues. Package names often discourage underscores, but for variables tenantname, for example, would typically be tenant_name, but this is fine too. These may not be the case throughout the library, and consistency is more important.
Looks good to me. Edits are up to you. I'll be anxious to hear feedback from the maintainers.
| self.log("Making a REST PUT request with headers {}".format(headers), level='debug') | ||
| else: | ||
| request_type = 'POST' | ||
| self.log("Making a REST POST request with headers {}".format(headers), level='debug') |
There was a problem hiding this comment.
For starters, this is indented by two spaces which should be corrected to four. Next, the third parameter to setattr, lambda: 'PUT' doesn't really make sense to me. Why not just setattr(url_request, 'get_method', 'PUT')?
|
|
||
| response = self.manager._request(rest_call, auth_required=False) | ||
|
|
||
| return response |
There was a problem hiding this comment.
This is one of those trivial things, but if response isn't used beyond the return, why not return the call to _request rather than setting a variable?
| core.CoreObject.__init__(self) | ||
| self.manager = manager | ||
| self.log = self.manager.log if self.manager else None | ||
| if api_response: self._set_properties(api_response, log_func) |
There was a problem hiding this comment.
Coding style question for the maintainers, but I think the newline makes this more clear unless using the full ternary.
Hi,
I've extended your sdk with support for adding/ getting tenants, tenant_templates - please merge into your ds10 branch, if you'd like.
Let me know of any updates/ issues/ questions - thanks for creating/ maintaining!
John