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

[Namespace]: Simplify sync_d functions to use higher order #154

Merged

Conversation

SuvarnaMeenakshi
Copy link
Contributor

functions.

Signed-off-by: SuvarnaMeenakshi sumeenak@microsoft.com

- What I did
Namespace class was added to support getting data from multiple namespaces.
This class includes some functions which get data from multiple namespaces and combine them.
This PR is to simplify the function of getting from separate functions and merge them.

- How I did it
Write a higher order function to invoke individual function to retrieve data from a single namespace.
The higher order function will retrieve the data and combine.

- How to verify it
No change in single and multi-asic snmp walk outputs.
Only implementation changes.
Make sure snmp walk outputs before change and after change are the same.

- Description for the changelog

functions.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
result_map[idx].update(ns_tuple[idx])
for idx, ns_tuple_dict in result_map.items():
result_list.append(ns_tuple_dict)
return(result_list)
Copy link
Contributor

@qiluo-msft qiluo-msft Jul 28, 2020

Choose a reason for hiding this comment

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

Suggested change
return(result_list)
return result_list
``` #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as per comment.

"""
Namespace.connect_namespace_dbs(dbs)
def get_sync_d_from_all_namespace(per_namespace_func, dbs):
# return list of dictionaries retrieved from per namespace functions
Copy link
Contributor

@qiluo-msft qiluo-msft Jul 28, 2020

Choose a reason for hiding this comment

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

return list of dictionaries [](start = 10, length = 27)

You did not return list of dictionaries #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we return list of dictionaries, the dictionaries will be the same as return from individual sync_d functions, just that they are merged together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably, we can say "return tuple of dictionaries"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated as per review comment.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 90e9f2e into sonic-net:master Aug 3, 2020
abdosi pushed a commit that referenced this pull request Sep 2, 2020
* [Namespace]: Simplify sync_d functions to use higher order
functions.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

* Update as per review comments.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants