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

[staticroutebfd] fix an error in error logging #17043

Merged
merged 2 commits into from Jan 31, 2024

Conversation

baorliu
Copy link
Contributor

@baorliu baorliu commented Oct 30, 2023

Why I did it

Fix an error in the log_err call.
this error can be triggered by an invalid static route key. usually the code cannot go here with normal config file. but hit this issue with an invalid key by manual testing with redis-cli directly. the file is scanned by Python lint to prevent such errors.

Work item tracking
  • Microsoft ADO ():26250268

How I did it

fix the format error.

How to verify it

1, ran pylint to check the design, make sure no such error in the design file.
2, wrote a separate python program to verify the log call.
In the current logging related testing, usually use patch/mock for logging. for this specific error, could not trigger it if we call mock function instead the real function in the design. so need to do lint checking for code change.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Fix an error in the log_err call.
this error can be triggered by an invalid static route key. usually the code cannot go here with normal config file. but hit this issue with an invalid key by manual testing with redis-cli directly.
the file is scanned by Python lint to prevent such errors.
prsunny
prsunny previously approved these changes Nov 2, 2023
@prsunny
Copy link
Contributor

prsunny commented Nov 2, 2023

@abdosi to review. @baorliu , is it possible to add a unit test?

@baorliu
Copy link
Contributor Author

baorliu commented Nov 2, 2023

@abdosi to review. @baorliu , is it possible to add a unit test?

Yes. I will check if I can use mocked function to test this logging issue.
[update] added a UT for the invalid key.
UT output without the fix:

===================================================================================================================================== FAILURES =====================================================================================================================================
_________________________________________________________________________________________________________________________________ test_invalid_key _________________________________________________________________________________________________________________________________
...

tests/test_static_rt_bfd.py:180: 
...
        if not valid:
>           log_err("invalid ip prefix for static route: ", key)
E           TypeError: log_err() takes 1 positional argument but 2 were given

staticroutebfd/main.py:379: TypeError

UT passed with the fix.

add UT for an invalid key logging issue.
@abdosi
Copy link
Contributor

abdosi commented Dec 14, 2023

LGTM. @StormLiangMS can you help with merge of this.

@anamehra
Copy link
Contributor

required in 202305/202311

@lguohan
Copy link
Collaborator

lguohan commented Dec 15, 2023

@abdosi , MS ADO?

@abdosi
Copy link
Contributor

abdosi commented Jan 31, 2024

@abdosi , MS ADO?

updated.

@lguohan lguohan merged commit d1cce42 into sonic-net:master Jan 31, 2024
22 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Jan 31, 2024
Why I did it
Fix an error in the log_err call.
this error can be triggered by an invalid static route key. usually the code cannot go here with normal config file. but hit this issue with an invalid key by manual testing with redis-cli directly. the file is scanned by Python lint to prevent such errors.

Work item tracking
Microsoft ADO ():26250268

How I did it
fix the format error.

How to verify it
1, ran pylint to check the design, make sure no such error in the design file.
2, wrote a separate python program to verify the log call.
In the current logging related testing, usually use patch/mock for logging. for this specific error, could not trigger it if we call mock function instead the real function in the design. so need to do lint checking for code change.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #17961

mssonicbld pushed a commit that referenced this pull request Jan 31, 2024
Why I did it
Fix an error in the log_err call.
this error can be triggered by an invalid static route key. usually the code cannot go here with normal config file. but hit this issue with an invalid key by manual testing with redis-cli directly. the file is scanned by Python lint to prevent such errors.

Work item tracking
Microsoft ADO ():26250268

How I did it
fix the format error.

How to verify it
1, ran pylint to check the design, make sure no such error in the design file.
2, wrote a separate python program to verify the log call.
In the current logging related testing, usually use patch/mock for logging. for this specific error, could not trigger it if we call mock function instead the real function in the design. so need to do lint checking for code change.
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Feb 2, 2024
Why I did it
Fix an error in the log_err call.
this error can be triggered by an invalid static route key. usually the code cannot go here with normal config file. but hit this issue with an invalid key by manual testing with redis-cli directly. the file is scanned by Python lint to prevent such errors.

Work item tracking
Microsoft ADO ():26250268

How I did it
fix the format error.

How to verify it
1, ran pylint to check the design, make sure no such error in the design file.
2, wrote a separate python program to verify the log call.
In the current logging related testing, usually use patch/mock for logging. for this specific error, could not trigger it if we call mock function instead the real function in the design. so need to do lint checking for code change.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #17985

mssonicbld pushed a commit that referenced this pull request Feb 2, 2024
Why I did it
Fix an error in the log_err call.
this error can be triggered by an invalid static route key. usually the code cannot go here with normal config file. but hit this issue with an invalid key by manual testing with redis-cli directly. the file is scanned by Python lint to prevent such errors.

Work item tracking
Microsoft ADO ():26250268

How I did it
fix the format error.

How to verify it
1, ran pylint to check the design, make sure no such error in the design file.
2, wrote a separate python program to verify the log call.
In the current logging related testing, usually use patch/mock for logging. for this specific error, could not trigger it if we call mock function instead the real function in the design. so need to do lint checking for code change.
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.

None yet

8 participants