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.

Sign up for GitHub

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

Jump to bottom

Made changes to aci_rest module to only add annotation when the value is a dictionary and changed the way the cookie is retrieved in the httpapi plugin (DCNE-176) #689

Merged
merged 1 commit into from
Oct 17, 2024
Merged

Made changes to aci_rest module to only add annotation when the value is a dictionary and changed the way the cookie is retrieved in the httpapi plugin (DCNE-176) #689

merged 1 commit into from
Oct 17, 2024

Conversation

shrsr
Copy link
Collaborator

@shrsr shrsr commented Oct 3, 2024

No description provided.

@shrsr shrsr self-assigned this Oct 3, 2024
@shrsr shrsr linked an issue Oct 3, 2024 that may be closed by this pull request
aci_rest needs to support disabling Intersight Connectivity (DCNE-176) #685
Closed
@github-actions github-actions bot changed the title Made changes to aci_rest module to only add annotation when the value is a dictionary and changed the way the cookie is retrieved in the httpapi plugin Made changes to aci_rest module to only add annotation when the value is a dictionary and changed the way the cookie is retrieved in the httpapi plugin (DCNE-176) Oct 3, 2024
@shrsr shrsr force-pushed the intersight branch 2 times, most recently from 6dc18b8 to bcd3f89 Compare October 4, 2024 14:26
akinross
akinross reviewed Oct 7, 2024
View reviewed changes
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

How do we handle the output when imdata does not exist in response? I think there is another issue at hand where the response returned is not outputted properly. Did you run tests with the system api endpoint that response is visible in output?

@shrsr
Copy link
Collaborator Author

shrsr commented Oct 7, 2024

For now the only case we've seen where imdata is not in the response is when the instance of the response is of type list.
We do the following to cover that particular case:

if isinstance(jsondata, list):
            self.imdata = jsondata
            self.totalCount = len(jsondata)

With the above code the response is visible in the output when the System API endpoint is called upon.

@shrsr shrsr requested a review from akinross October 7, 2024 14:30
gmicol
gmicol requested changes Oct 7, 2024
View reviewed changes
plugins/module_utils/aci.py Show resolved Hide resolved
anvitha-jain
anvitha-jain previously approved these changes Oct 7, 2024
View reviewed changes
Copy link
Collaborator

@anvitha-jain anvitha-jain left a comment

Choose a reason for hiding this comment

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

LGTM

@shrsr
[bugfix] Made changes to aci_rest module to only add annotation when …
f9d284f 
…the value is a dictionary and changed the way the cookie is retrieved in the httpapi plugin
akinross
akinross approved these changes Oct 8, 2024
View reviewed changes
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

@lhercot lhercot requested review from gmicol and anvitha-jain October 8, 2024 11:06
sajagana
sajagana approved these changes Oct 8, 2024
View reviewed changes
Copy link
Collaborator

@sajagana sajagana left a comment

Choose a reason for hiding this comment

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

LGTM!

gmicol
gmicol approved these changes Oct 8, 2024
View reviewed changes
Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

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

LGTM

anvitha-jain
anvitha-jain approved these changes Oct 8, 2024
View reviewed changes
Copy link
Collaborator

@anvitha-jain anvitha-jain left a comment

Choose a reason for hiding this comment

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

LGTM

samiib
samiib requested changes Oct 9, 2024
View reviewed changes
tests/integration/targets/aci_rest/tasks/json_inline.yml Show resolved Hide resolved
plugins/httpapi/aci.py Show resolved Hide resolved
plugins/httpapi/aci.py Show resolved Hide resolved
@shrsr shrsr requested a review from samiib October 10, 2024 18:41
samiib
samiib approved these changes Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM

samiib
samiib approved these changes Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM

lhercot
lhercot approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@lhercot lhercot left a comment

Choose a reason for hiding this comment

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

LGTM

@lhercot lhercot merged commit fe4ba0d into CiscoDevNet:master Oct 17, 2024
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmicol gmicol gmicol approved these changes

@samiib samiib samiib approved these changes

@sajagana sajagana sajagana approved these changes

@anvitha-jain anvitha-jain anvitha-jain approved these changes

@akinross akinross akinross approved these changes

@lhercot lhercot lhercot approved these changes

Assignees

@shrsr shrsr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

aci_rest needs to support disabling Intersight Connectivity (DCNE-176)
7 participants
@shrsr @lhercot @akinross @anvitha-jain @sajagana @samiib @gmicol