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

route_check.py should not print too many outputs to stdout #3071

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

MaxXiao12
Copy link
Contributor

@MaxXiao12 MaxXiao12 commented Dec 7, 2023

Otherwise it fills up the pipe and print() will block. When this happens, route_check.py will timeout and eventually killed by monit after several minutes.

Request to cast the change to 202305 branch.

What I did

Add an option in print_message() to skip printing.

How I did it

Add an option write_to_stdout=True, in mitigate_installed_not_offloaded_frr_routes() call print_message() with write_to_stdout=False.

How to verify it

Manually run route_check.py, verify that the messages were shown in syslog but not in stdout.

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)

Otherwise it fills up the pipe and print() will block.
Copy link

linux-foundation-easycla bot commented Dec 7, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@StormLiangMS
Copy link
Contributor

@dgsudharsan @prsunny would you help to review and comment?

@prsunny
Copy link
Contributor

prsunny commented Dec 11, 2023

Can you please check on coverage? Changes lgtm

@dgsudharsan
Copy link
Collaborator

Changes LGTM. Please add coverage as requested by Prince

@MaxXiao12
Copy link
Contributor Author

Can you please check on coverage? Changes lgtm

Hi Prince, I wasn't able to add labels to the PR to indicate status and also which branch to cast to... Could you check if I am included in the right group? I am working on the test in the meantime to close the gap.

Thanks!

@prsunny
Copy link
Contributor

prsunny commented Dec 13, 2023

@lolyu for viz

@StormLiangMS
Copy link
Contributor

Can you please check on coverage? Changes lgtm

Hi Prince, I wasn't able to add labels to the PR to indicate status and also which branch to cast to... Could you check if I am included in the right group? I am working on the test in the meantime to close the gap.

Thanks!

Hi @MaxXiao12 you can add request in the description of PR. The maintainer or release mgr will add tags.

@MaxXiao12
Copy link
Contributor Author

Can you please check on coverage? Changes lgtm

Hi Prince, I wasn't able to add labels to the PR to indicate status and also which branch to cast to... Could you check if I am included in the right group? I am working on the test in the meantime to close the gap.
Thanks!

Hi @MaxXiao12 you can add request in the description of PR. The maintainer or release mgr will add tags.

Updated the PR to request for 202305 branch

@prsunny
Copy link
Contributor

prsunny commented Dec 15, 2023

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft qiluo-msft merged commit 19ea849 into sonic-net:master Dec 15, 2023
5 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Dec 20, 2023
…#3071)

Otherwise it fills up the pipe and print() will block. When this happens, `route_check.py` will timeout and eventually killed by `monit` after several minutes.

#### What I did
Add an option in `print_message()` to skip printing.

#### How I did it
Add an option `write_to_stdout=True`, in `mitigate_installed_not_offloaded_frr_routes()` call `print_message()` with `write_to_stdout=False`.

#### How to verify it
Manually run route_check.py, verify that the messages were shown in syslog but not in stdout.

#### 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)
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #3092

mssonicbld pushed a commit that referenced this pull request Dec 20, 2023
Otherwise it fills up the pipe and print() will block. When this happens, `route_check.py` will timeout and eventually killed by `monit` after several minutes.

#### What I did
Add an option in `print_message()` to skip printing.

#### How I did it
Add an option `write_to_stdout=True`, in `mitigate_installed_not_offloaded_frr_routes()` call `print_message()` with `write_to_stdout=False`.

#### How to verify it
Manually run route_check.py, verify that the messages were shown in syslog but not in stdout.

#### 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)
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.

8 participants