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

Fix for Rest Connector for nd (Nexus Dashboard) will not work for remote authentication #132

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

vijperum
Copy link

@vijperum vijperum commented Aug 9, 2024

Redacted default password in source code.
Fix for domain in testbed file made it optional so that it backward compatible.
Added get, post, put, delete with params, data, files, json and removed expected_status_code and return type requests.Response

#131

…tion made domain optional in connections so that when domain is not there in testbed it defaults to DefaultAuth. And if any new domain is added then value can be specified in testbed.yaml
Overloaded get, post, put and delete so that they are backward compatible.
@vijperum vijperum requested a review from a team as a code owner August 9, 2024 15:05
src/rest/connector/libs/nd/implementation.py Outdated Show resolved Hide resolved
src/rest/connector/libs/nd/implementation.py Outdated Show resolved Hide resolved
"""connect to the device via REST
Arguments
---------
timeout (int): Timeout value
retries (int): Max retries on request exception (default: 3)
retry_wait (int): Seconds to wait before retry (default: 10)
verify (bool): defaults to False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Defaults to False, but what does it do? A brief description of what we're verifying would be ideal

src/rest/connector/libs/nd/implementation.py Outdated Show resolved Hide resolved
Comment on lines -231 to -232
log.info("Output received:\n{output}".format(output=
json.dumps(output, indent=2, sort_keys=True)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're removing the dump? It'll make reading the output harder

Comment on lines 454 to 460
api_url (string): subdirectory part of the API URL
params (dict): Query string parameters
data (dict):
json (json) : if request header Content-Type is application/json
files (dict):
expected_status_code (int): Expected result
timeout (int): Maximum time
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add defaults

src/rest/connector/libs/nd/implementation.py Outdated Show resolved Hide resolved
@@ -423,49 +513,97 @@ def delete(self, api_url, expected_status_code=requests.codes.ok,
timeout (int): Maximum time
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure to update the docstring

Comment on lines 562 to 567
api_url (string): subdirectory part of the API URL
params (dict): Query string parameters
data (dict):
json (json) : if request header Content-Type is application/json
files (dict):
timeout (int): Maximum time
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add defaults

Comment on lines 580 to 596
if params and not json:
response = self.session.delete(full_url, params=params, timeout=timeout,
verify=self.verify)
elif params and json:
response = self.session.delete(full_url, params=params, json=json,
timeout=timeout, verify=self.verify)
elif json and not params:
response = self.session.delete(full_url, json=json,
timeout=timeout, verify=self.verify)
elif data:
response = self.session.delete(full_url, data=data,
timeout=timeout, verify=self.verify)
elif files:
response = self.session.delete(full_url, files=files,
timeout=timeout, verify=self.verify)
else:
response = self.session.delete(full_url, timeout=timeout, verify=self.verify)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

vijperum and others added 5 commits August 23, 2024 20:17
Updated doc for verify
Addressed Are we not able to pass None to the values? IE, surely if json is None we can still do self.session.post(..., json=None)?

Would be better than having this large if/else structure
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.

2 participants