Allow target-level control parameter overrides #103
Conversation
|
Requires ceph/ceph-iscsi-config#66 |
gwcli/gateway.py
Outdated
| else: | ||
| self.logger.error("Failed to reconfigure : " | ||
| "{}".format(response_message(api.response, | ||
| self.logger))) |
There was a problem hiding this comment.
I am not sure where yet, but I think we need a _set_controls/_refresh_control_values call somewhere around this time, so if the info command is run after the reconfigure then you see the updated values. Or maybe _get_controls should do a _refresh_control_values, so Target.refresh() calls update those values.
I noticed for disk reconfigure operations we do it in the reconfigure function when successful which I think is in a similar place as here.
gwcli/gateway.py
Outdated
| sorted(settings.Settings.GATEWAY_SETTINGS)))) | ||
| return | ||
|
|
||
| # Issue the api request for the resize |
There was a problem hiding this comment.
I think change "resize" to "reconfigure".
gwcli/gateway.py
Outdated
| def summary(self): | ||
| return "Gateways: {}".format(len(self.gateway_group.children)), None | ||
|
|
||
| def ui_command_info(self): | ||
| self.logger.debug("CMD: info") | ||
|
|
There was a problem hiding this comment.
Should the Target class be UINode instead of UIGroup, so we can use the info helpers? We can then drop all this and just add a display_attributes arrary with target_iqn and control_values and this function is just a call to
console_message(get_info())
?
I honestly did not understand the difference between uigroup and uinode other than the info difference, so no idea if that is ok wrt python/object oriented design. It just looked easier code wise.
My last review comment. Sorry for the sporadic review comments over multiple days :)
There was a problem hiding this comment.
From my quick glance, I thought the UIGroup is allowed to have children and the UINode was designed to be a leaf. I might be wrong, though.
There was a problem hiding this comment.
... never mind, they seem to basically be the same thing. updated.
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
| controls = {} | ||
| raw_controls = json.loads(controls_json) | ||
| for k in settings.Settings.GATEWAY_SETTINGS: | ||
| if raw_controls.has_key(k): |
There was a problem hiding this comment.
has_key has been deprecated in favor of 'in' (I think).
See http://portingguide.readthedocs.io/en/latest/dicts.html
There was a problem hiding this comment.
This is already merged, so feel free to open a PR to incorporate your suggestions.
| raw_controls = json.loads(controls_json) | ||
| for k in settings.Settings.GATEWAY_SETTINGS: | ||
| if raw_controls.has_key(k): | ||
| value = raw_controls[k] if raw_controls[k] else None |
There was a problem hiding this comment.
Perhaps:
value = raw_controls.get(k) ?
No description provided.