Clean pr base with the wireless design #161
Conversation
rukapse
left a comment
rukapse
left a comment
There was a problem hiding this comment.
Reviewed the code, a lot of the comments would repeat below.
Make to avoid very large functions
use the get functions already in the dnac.py file
for large if/elifs use the mapping mechanism
make sure exception logging includes all the details along with the error
Otherwise, documentation is really good. But, within documentation if defaults are there make sure to add them in the documentation, and for booleans as well.
| - RRM considers optimization when throughput falls below this value. | ||
| type: int | ||
| required: false | ||
| coverage_hole_detection: |
There was a problem hiding this comment.
what is the default for coverage_hole_detection? is it true? If True add it to the documentation.
There was a problem hiding this comment.
addressed review comments
| required: false | ||
| suboptions: | ||
| clean_air: | ||
| description: |
There was a problem hiding this comment.
if default for clean_air is True add it, for other boolean values also add it if true
There was a problem hiding this comment.
addressed review comments
| - Controls sensitivity for detecting overlapping BSS transmissions. | ||
| - Range typically -82 to -62 dBm. | ||
| type: int | ||
| target_wake_up_time_11ax: |
There was a problem hiding this comment.
could be -> target_wakeup_time_11ax, ignore if too much work.
There was a problem hiding this comment.
addressed review comments
| description: | ||
| - Radio frequency band for Event-Driven RRM operation. | ||
| - RRM algorithms will monitor and optimize this band. | ||
| - Note that 6GHz band is not supported for Event-Driven RRM. |
There was a problem hiding this comment.
Note - Currently, 6 GHz band is not supported for Event-Driven RRM
There was a problem hiding this comment.
addressed review comments
| - When enabled, system responds to interference and performance changes. | ||
| type: bool | ||
| required: false | ||
| event_driven_rrm_threshold_level: |
There was a problem hiding this comment.
there is no default as its an optional field
| "event_driven_rrm_enable": "eventDrivenRrmEnable", | ||
| "event_driven_rrm_threshold_level": "eventDrivenRrmThresholdLevel", | ||
| "event_driven_rrm_custom_threshold_val": "eventDrivenRrmCustomThresholdVal", | ||
| } |
|
|
||
| return delete_list | ||
|
|
||
| def verify_delete_dot11axs_requirement(self, dot11ax_list): |
There was a problem hiding this comment.
verify_delete_dot11axs_requirement
is it supposed to be verify_delete_dot11ax_requirement without s? ignore if it is axs
| # warn user / playbook author that some unlocked attrs were invalid and dropped | ||
| self.log( | ||
| "Some unlockedAttributes were invalid and removed for '{0}': dropped={1}, unmapped={2}".format( | ||
| design_name, dropped_unlocked, unmapped_unlocked |
There was a problem hiding this comment.
add lengths also
[{design_name}] dropped={len(dropped_unlocked)}, unmapped={len(unmapped_unlocked)}
| self.log("dot11ax to ADD: {0}, UPDATE: {1}, NO-UPDATE: {2}".format(len(add_list), len(update_list), len(no_update_list)), "DEBUG") | ||
| return add_list, update_list, no_update_list | ||
|
|
||
| def get_dot11ax_details(self, template_id): |
There was a problem hiding this comment.
again see if you can use the exucute get request from dnac.py
sample -> reduces code lines
def get_interfaces(self, get_interfaces_params):
"""
Retrieves interface details using pagination.
Args:
get_interfaces_params (dict, optional): Parameters for filtering the interfaces. Defaults to an empty dictionary.
Returns:
list: A list of dictionaries containing details of interfaces based on the filtering parameters.
"""
self.log(
"Retrieving interfaces with parameters: {0}".format(get_interfaces_params),
"INFO",
)
# Execute the paginated API call to retrieve interfaces
return self.execute_get_with_pagination(
"wireless", "get_interfaces", get_interfaces_params
)
|
|
||
| return delete_clean_air_list | ||
|
|
||
| def verify_create_update_clean_air_requirement(self, clean_air_list, field_to_check=None): |
There was a problem hiding this comment.
This function is too long, try splitting it up
maybe with helpers like this
_normalize_clean_air_payload()
_compare_clean_air_fields()
_compare_nested_interferers()
3 participants
Type of Change
Description
Added CRUD handlers for wireless feature templates (AAA Radius, Advanced SSID, CleanAir, Dot11ax, Dot11be, Event-driven RRM, FlexConnect, Multicast, RRM FRA, RRM General).
Enhancement:
Summary
Added (Create/Update/Delete) for each wireless feature template below
Included templates
---There were no deletions from my side, There is a issue with git commit---
Enhancement Description: [Explain what was enhanced, why, and how]
Impact Area: [Mention which part of the system/codebase is affected]
Testing Done:
Test cases covered: [Mention test case IDs or brief points]
Checklist
Ansible Best Practices
ansible-vaultor environment variables)Documentation
Screenshots (if applicable)
Notes to Reviewers