Conversation
super-cooper
left a comment
super-cooper
left a comment
There was a problem hiding this comment.
Code is good, and fits in nicely with the rest of blazarclient! There are a few very small things which I think could use come cleaning up.
blazarclient/command.py
Outdated
| if hasattr(parsed_args, "resource") and hasattr(blazar_client, parsed_args.resource): | ||
| resource_manager = getattr(blazar_client, parsed_args.resource) | ||
| if add_to_body: | ||
| args["resource_type"] = self.resource | ||
| elif hasattr(parsed_args, "resource"): # If third party resource | ||
| resource_manager = blazar_client.resource | ||
| if add_to_body: | ||
| args["resource_type"] = parsed_args.resource | ||
| else: | ||
| args.insert(0, parsed_args.resource) |
There was a problem hiding this comment.
This branching is a little difficult to parse at first glance. I prefer consolidating a little bit
if hasattr(parsed_args, "resource"):
if hasattr(blazar_client, parsed_args.resource)
resource_manager = getattr(blazar_client, parsed_args.resource)
resource_type = self.resource
else:
resource_manager = blazar_client.resource
resource_type = parsed_args.resource
if add_to_body:
args["resource_type"] = resource_type
blazarclient/command.py
Outdated
| resource_manager = getattr(blazar_client, self.resource) | ||
| resource_manager.update(res_id, **body) | ||
| resource_manager, args = self.get_manager_and_args(parsed_args, [res_id], add_to_body=False, blazar_client=blazar_client) | ||
| data = resource_manager.update(*args, **body) |
blazarclient/command.py
Outdated
| self.log.debug('run(%s)' % parsed_args) | ||
| blazar_client = self.get_client() | ||
| resource_manager = getattr(blazar_client, self.resource) | ||
| res_id = parsed_args.id |
There was a problem hiding this comment.
This gets overwritten immediately if self.allow_names is true. It should probably be in an else branch to be more clear.
blazarclient/v1/resources.py
Outdated
| def list_resources(self, sort_by=None): | ||
| resp, body = self.request_manager.get('/resources') | ||
| if sort_by: | ||
| resources = sorted(body, key=lambda l: l[sort_by]) |
There was a problem hiding this comment.
This variable is unused, and sorted produces a copy, so nothing will actually be sorted to the user.
| import logging | ||
| LOG = logging.getLogger(__name__) | ||
|
|
||
| class ResourceClientManager(base.BaseClientManager): |
There was a problem hiding this comment.
Most of the dict lookups in this class assume that the server will also have plugins installed. Is this assumption safe to make?
There was a problem hiding this comment.
I think this is fine, as blazar already assumes you have oshosts/network/fip running. There is a /resources endpoint in 3rd party plugins patch which shows enabled resources we could check, but that would require an extra API call for each command.
blazarclient/v1/resources.py
Outdated
| return body['resource'] | ||
|
|
||
| def update(self, resource_type, res_id, data, extras): | ||
| LOG.info("RESOURCE CLIENT UPDATE") |
| return body['resource'] | ||
|
|
||
| def delete(self, resource_type, resource_id): | ||
| resp, body = self.request_manager.delete( |
2 participants
No description provided.