-
Notifications
You must be signed in to change notification settings - Fork 644
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
Upgrade pyroute2 and improve cli response time #3513
base: master
Are you sure you want to change the base?
Conversation
50e311a
to
d48074b
Compare
@qiluo-msft Requesting waiver to ignore the coverage failure. I tried adding an UT to cover the multi_asic_get_ip_intf_from_ns and multi_asic_get_ip_intf_addr_from_ns methods (50e311a) .
This is because the monkey patched API is called instead of the actual API
Thus i couldn't add a UT to cover the lines i've added. This PR just changes the location of import and so no major functionality change that warrants a UT |
Thanks for the UT effort! Could you further explore if you can just disable multi_asic_get_ip_intf_from_ns monkey patch in your specific testcase? We should not globally enable one function monkey patch, because it will prevent us to UT the the function itself. In reply to: 2330382035 |
Hi, i checked but monkey patching is global. once the file is imported and loaded, the original api is lost. I suppose this can be updated but changing this would also probably require some other Multi Asic UT's to also be updated and so i think this should be taken as a separate item |
What I did
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)