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

Support validating nested list of dictionary as input, make base clas… #7

Merged
merged 8 commits into from
Sep 22, 2023

Conversation

Abhishek-121
Copy link
Collaborator

Support validating nested list of dictionary as input, make base class as Abstract class with validate function, make the required changes in template_intent module based on DnacBase class and remove redundant code, with supported states as well.

…s as Abstract class with validate function, make the required changes in template_intent module based on DnacBase class and remove redundant code, with supported states as well
@madhansansel madhansansel self-requested a review September 11, 2023 12:10
@@ -43,10 +44,17 @@ def __init__(self, module):
self.get_diff_state_apply = {'merged': self.get_diff_merged,
Copy link
Owner

Choose a reason for hiding this comment

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

Can we also write for other states?
"merged" : self.get_diff_merged
"replaced" : self.get_diff_replaced
and all..
Also write member functions for all the states..

self.result = {"changed": False, "diff": [], "response": [], "warnings": []}

@abstractmethod
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need to write function body for abstractmethod? What is the use of it? Anyhow we are going to override it in base class..

@@ -57,7 +65,7 @@ def log(self, message, frameIncrement=0):
def check_return_status(self):
"""API to check the return status value and exit/fail the module"""

self.log("status: {0}, msg:{1}".format(self.status, self.msg), frameIncrement=1)
log("status: {0}, msg:{1}".format(self.status, self.msg), frameIncrement=1)
Copy link
Owner

Choose a reason for hiding this comment

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

Any reason for changing this into self.log? Because if dnac.log is true, then only we call log API to print.

@@ -88,7 +96,7 @@ def get_task_details(self, task_id):
params={"task_id": task_id},
)

self.log(str(response))
log(str(response))
Copy link
Owner

Choose a reason for hiding this comment

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

Any reason for changing this into self.log? Because if dnac.log is true, then only we call log API to print.

@@ -202,6 +210,83 @@ def dnac_argument_spec():
)
return argument_spec

def validate_str(item, param_spec, param_name, invalid_params):
item = validation.check_type_str(item)
Copy link
Owner

Choose a reason for hiding this comment

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

Can we write Doc string for this new API ?

return validation.check_type_bool(item)

def validate_list(item, param_spec, param_name, invalid_params):
try:
Copy link
Owner

Choose a reason for hiding this comment

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

Doc String for this API ..

def validate_list(item, param_spec, param_name, invalid_params):
try:
if param_spec.get("type") == type(item).__name__:
if param_spec:
Copy link
Owner

Choose a reason for hiding this comment

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

No need of this check.. If param_spec is already there and get the type before we check it..
As we have lot more statements in this if block, we can check with not equal and return in the beginning itself.

keys_list.append(dict_key)
if len(keys_list) == 1:
return validation.check_type_list(item)
else:
Copy link
Owner

Choose a reason for hiding this comment

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

as we are returning from if block unconditional, do we need else here?

if type(element).__name__ != get_spec_element:
invalid_params.append(
"{0} is not of the same datatype as expected which is {1}".format(
element, get_spec_element
Copy link
Owner

Choose a reason for hiding this comment

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

can we close the braces in the same line itself becuase adding multiple new lines only for braces looks little messier ?

return item

def validate_dict(item, param_spec, param_name, invalid_params):
if param_spec.get("type") != type(item).__name__:
Copy link
Owner

Choose a reason for hiding this comment

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

Doc String for this new API

@madhansansel madhansansel merged commit d0d41a4 into madhansansel:main Sep 22, 2023
madhansansel pushed a commit that referenced this pull request Jan 31, 2024
bug fix for when dnac_log_level not specified in the playbook
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