Skip to content
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

unity fix the issue which when filesystem is None in get_share #696

Merged
merged 32 commits into from
Oct 15, 2021

Conversation

tanjiangyu-ghca
Copy link
Contributor

@tanjiangyu-ghca tanjiangyu-ghca commented Sep 16, 2021

What this PR does / why we need it:
1.fix the issue which when filesystem is None in get
2.fix some compatibility issue
3.change fcport localtion and connect_status
4.change get alarm's timeout
5.change port speed and disk name
6.remove the disk when it's slot is empty
7.change disk physicaltype

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:

@codecov
Copy link

codecov bot commented Sep 16, 2021

Codecov Report

Merging #696 (756a576) into master (6ad2d83) will increase coverage by 0.19%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master     #696      +/-   ##
==========================================
+ Coverage   70.22%   70.41%   +0.19%     
==========================================
  Files         163      163              
  Lines       15627    15836     +209     
  Branches     1939     1960      +21     
==========================================
+ Hits        10974    11151     +177     
- Misses       3992     4010      +18     
- Partials      661      675      +14     
Impacted Files Coverage Δ
delfin/drivers/dell_emc/unity/alert_handler.py 81.48% <33.33%> (-1.86%) ⬇️
delfin/drivers/dell_emc/unity/unity.py 68.42% <36.53%> (-3.66%) ⬇️
delfin/drivers/dell_emc/unity/rest_handler.py 48.85% <52.63%> (-1.15%) ⬇️
delfin/drivers/dell_emc/unity/consts.py 100.00% <100.00%> (ø)
delfin/drivers/utils/rest_client.py 38.88% <100.00%> (ø)
delfin/api/middlewares.py 100.00% <0.00%> (ø)
delfin/api/v1/access_info.py 100.00% <0.00%> (ø)
delfin/api/validation/__init__.py 100.00% <0.00%> (ø)
delfin/exporter/prometheus/exporter_server.py 0.00% <0.00%> (ø)
...dulers/telemetry/performance_collection_handler.py 100.00% <0.00%> (ø)
... and 48 more

@tanjiangyu-ghca tanjiangyu-ghca changed the title fix the issue which when filesystem is None in get_share unity fix the issue which when filesystem is None in get_share Sep 16, 2021
NajmudheenCT
NajmudheenCT previously approved these changes Sep 27, 2021
Copy link
Member

@NajmudheenCT NajmudheenCT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -81,44 +83,47 @@ def login(self):
LOG.error("Login error: %s", six.text_type(e))
raise e

def call_with_token(self, url, data, method):
def call_with_token(self, url, data, method, calltimeout):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data can set default ot None, method default to GET, calltimeout default to consts.DEFAULT_TIMEOUT.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

self.call(RestHandler.REST_LOGOUT_URL, method='POST')
self.call(RestHandler.REST_LOGOUT_URL,
consts.DEFAULT_TIMEOUT,
data={}, method='POST')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter data can use default value of method call().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if res.status_code == 200:
result_json = res.json()
return result_json

def call(self, url, data=None, method=None):
def call(self, url, calltimeout, data=None, method=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the parameters of call() is the same as the parameters of call_with_token(), these parameters should be in the same order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

'messageId,message,description,'
'descriptionId',
page_number)
result_json = self.get_rest_info(url, consts.ALERT_TIMEOUT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second parameter of get_rest_info() is data, not calltimeout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

auth_key = None
if self.session:
auth_key = self.session.headers.get(RestHandler.AUTH_KEY, None)
if auth_key:
self.session.headers[RestHandler.AUTH_KEY] \
= cryptor.decode(auth_key)
res = self.do_call(url, data, method)
res = self.do_call(url, data, method, calltimeout)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value of parameter calltimeout in do_call() is from driver code of HPE 3par, method do_call() is a common method, I think its default value should be also from common module or from its own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator

@ThisIsClark ThisIsClark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@ThisIsClark ThisIsClark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@NajmudheenCT NajmudheenCT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@NajmudheenCT NajmudheenCT merged commit de5c139 into sodafoundation:master Oct 15, 2021
@tanjiangyu-ghca tanjiangyu-ghca deleted the unity_0916 branch October 25, 2021 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants