add msa system pool volume disks alerts resources #761
Conversation
Codecov Report
@@ Coverage Diff @@
## master #761 +/- ##
==========================================
- Coverage 70.60% 70.57% -0.03%
==========================================
Files 170 173 +3
Lines 17235 17643 +408
Branches 2492 2569 +77
==========================================
+ Hits 12168 12451 +283
- Misses 4341 4432 +91
- Partials 726 760 +34
|
| from delfin.drivers.utils.tools import Tools | ||
|
|
||
| try: | ||
| import xml.etree.cElementTree as ET |
There was a problem hiding this comment.
What is the difference between line10 and 12?
There was a problem hiding this comment.
The correct thing is except ImportError:
import xml.etree.ElementTree as ET I made a mistake in writing. I wrote an extra C
| if 'is not a recognized command' in result: | ||
| raise exception.InvalidIpOrPort() | ||
| except Exception as e: | ||
| LOG.error("Failed to login msa storwize_svc %s" % |
There was a problem hiding this comment.
Why there is storwize_svc in this log?
There was a problem hiding this comment.
Forget to delete the error log. It will be updated next time
| def get_storage(self, storage_id): | ||
| try: | ||
| result = self.ssh_pool.do_exec('show system') | ||
| list = self.analysisDataToList(result, 'system') |
There was a problem hiding this comment.
list is a type name of python, please do not use this as a variable name
There was a problem hiding this comment.
The specification of Python has been relearned, and this problem has been changed. It will be updated next time
|
|
||
| def get_storage(self, storage_id): | ||
| try: | ||
| result = self.ssh_pool.do_exec('show system') |
There was a problem hiding this comment.
result is a meaningless variable name, please change it
There was a problem hiding this comment.
The specification of Python has been relearned, and this problem has been changed. It will be updated next time
|
|
||
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) | ||
| self.ssh_hanlder = ssh_handler.SSHHandler(**kwargs) |
There was a problem hiding this comment.
Wrong spelling of word ssh_hanlder .
There was a problem hiding this comment.
The next version has been updated with parameters
| LOG = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class SSHHandler(): |
There was a problem hiding this comment.
The next version has been updated
| def login(self): | ||
| try: | ||
| pool_info = self.ssh_pool.do_exec('show pools') | ||
| if 'is not a recognized command' in pool_info: |
There was a problem hiding this comment.
Redundant code. This if block has already judged in self.ssh_pool.do_exec() .
There was a problem hiding this comment.
This judgment has been deleted
| version_info = self.ssh_pool.do_exec('show version') | ||
| version_arr = self.handle_detail(version_info, 'versions') | ||
| if version_arr: | ||
| version_id = version_arr[0]\ |
There was a problem hiding this comment.
Line feed has been canceled
| def parse_alert(alert): | ||
| try: | ||
| alert_model = dict() | ||
| alert_id = '' |
There was a problem hiding this comment.
Default values of below 4 local variable might should be None .
There was a problem hiding this comment.
change value to None
| alert_model['occur_time'] = int(round(now * SSHHandler. | ||
| SECONDS_TO_MS)) | ||
| alert_model['description'] = description | ||
| alert_model['resource_type'] = '' |
There was a problem hiding this comment.
If info cannot be attained from storage, it should be deleted.
There was a problem hiding this comment.
Delete non-existent fields
| alert_model['description'] = description | ||
| alert_model['resource_type'] = '' | ||
| alert_model['location'] = description | ||
| print(alert_model) |
There was a problem hiding this comment.
print syntax should be deleted in product environment.
| return alert_model | ||
| except Exception as e: | ||
| LOG.error(e) | ||
| msg = ("Failed to build alert model as some attributes missing " |
There was a problem hiding this comment.
How about msg = ("Failed to build alert model: %s.") % (six.text_type(e)) ?
There was a problem hiding this comment.
Prompt statement changed
| @staticmethod | ||
| def handle_split(split_str, split_char, arr_number): | ||
| split_value = '' | ||
| if split_str is not None and split_str != '': |
There was a problem hiding this comment.
How about if split_str: xxx ?
There was a problem hiding this comment.
judgment statement changed
| class Request: | ||
| def __init__(self): | ||
| self.environ = {'delfin.context': context.RequestContext()} | ||
| pass |
| } | ||
|
|
||
|
|
||
| class Request: |
There was a problem hiding this comment.
Please confirm this class has been used.
There was a problem hiding this comment.
delete this class function
| def get_storage(self, storage_id): | ||
| try: | ||
| system_info = self.ssh_pool.do_exec('show system') | ||
| system_data = self.handle_detail(system_info, 'system') |
There was a problem hiding this comment.
What would handle_detail do, this fucntion name is meaningless
There was a problem hiding this comment.
change the function name to handle_xml_to_json
| for pool in pool_arr: | ||
| total_capacity += int(pool.get('total_capacity')) | ||
| disk_arr = self.list_storage_disks(storage_id) | ||
| disk_all = 0 |
There was a problem hiding this comment.
Why named 'disk_all' instead of 'raw_capacity'?
There was a problem hiding this comment.
use raw_capacity instead of disk_all
| version_id = version_arr[0]\ | ||
| .get('bundle-version') | ||
| if system_data: | ||
| pool_arr = self.list_storage_pools(storage_id) |
There was a problem hiding this comment.
Variable name changed
| total_capacity = 0 | ||
| for pool in pool_arr: | ||
| total_capacity += int(pool.get('total_capacity')) | ||
| disk_arr = self.list_storage_disks(storage_id) |
There was a problem hiding this comment.
Variable name changed
| def list_storage_ports(self, storage_id): | ||
| try: | ||
| ports_info = self.ssh_pool.do_exec('show ports') | ||
| ports_arr = ports_info.split('\n') |
There was a problem hiding this comment.
What's the difference between ports_arr and ports_array
There was a problem hiding this comment.
Variable name changed
| for child in ch.iter("PROPERTY"): | ||
| msg[child.get('name')] = child.text | ||
| port_detail.append(msg) | ||
| list_all_ports = [] |
There was a problem hiding this comment.
What's the difference between list_all_ports and list_ports
There was a problem hiding this comment.
Variable name changed
| ports_info = self.ssh_pool.do_exec('show ports') | ||
| ports_arr = ports_info.split('\n') | ||
| ports_array = ports_arr[1:len(ports_arr) - 1] | ||
| ports_xml = ''.join(ports_array) |
There was a problem hiding this comment.
Variable name changed
| ports_xml = ''.join(ports_array) | ||
| xml_element = ET.fromstring(ports_xml) | ||
| port_detail = [] | ||
| for ch in xml_element.iter("OBJECT"): |
There was a problem hiding this comment.
Variable name changed
update vendor info,alert info
update alert info
update alert info time
What this PR does / why we need it:
add msa system pool volume disks alerts resources
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: