Skip to content

[bugfix] Fix sanitize function for nd_rest module (DCNE-684)#210

Open
gmicol wants to merge 2 commits intoCiscoDevNet:masterfrom
gmicol:nd_rest_bug_fix_181
Open

[bugfix] Fix sanitize function for nd_rest module (DCNE-684)#210
gmicol wants to merge 2 commits intoCiscoDevNet:masterfrom
gmicol:nd_rest_bug_fix_181

Conversation

@gmicol
Copy link
Copy Markdown
Collaborator

@gmicol gmicol commented Mar 30, 2026

fix #181

The sanitize function will now return an empty dict when the obj_to_sanitize is None: Deals with cases where the API returns None.

…when The API-returned object to sanitize is None.
Copy link
Copy Markdown
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

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

LGTM

"""Clean up a Python object of type list or dict from specific keys, values and None values if specified"""
if isinstance(obj_to_sanitize, dict):
if obj_to_sanitize is None:
return {}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why determine to return a dict and not a list because the function cleans list or dict? Why not return None in this case?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

None makes definitely more sense. Changed it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@gmicol I checked the modules to see where sanitize() is being used and looks like nd_rest is the only module using it so far. However, In nd_rest could you please verify whether we could replace:

if sanitize(nd.result["jsondata"], ND_REST_KEYS_TO_SANITIZE) != nd.previous and method != "GET":
    nd.result["changed"] = True

with

if nd.existing != nd.previous and method != "GET":
    nd.result["changed"] = True

@gmicol gmicol requested a review from akinross March 31, 2026 17:51
@shrsr shrsr self-requested a review April 1, 2026 01:28
Copy link
Copy Markdown
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

Copy link
Copy Markdown
Collaborator

@mikewiebe mikewiebe 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
Copy Markdown
Collaborator

@allenrobel allenrobel left a comment

Choose a reason for hiding this comment

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

LGTM

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.

nd_rest module crashes with TypeError when API returns empty response body (DCNE-684)

5 participants