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

[config reload] Fix invalid rstrip. #2157

Merged
merged 3 commits into from
May 12, 2022
Merged

[config reload] Fix invalid rstrip. #2157

merged 3 commits into from
May 12, 2022

Conversation

ganglyu
Copy link
Contributor

@ganglyu ganglyu commented May 10, 2022

Signed-off-by: Gang Lv [email protected]

What I did

'rstrip' behavior is wrong, if unit is 'gnmi.timer', the result is 'gn', and 'gnmi' is expected.

How I did it

Use 're.sub' to replace 'rstrip'.

How to verify it

Run unit test for sonic-utilities.

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)

@ganglyu ganglyu requested a review from qiluo-msft May 10, 2022 04:23
config/main.py Outdated
@@ -746,7 +746,7 @@ def _get_delayed_sonic_services():
services = []
for unit in timer:
if state[timer.index(unit)] == "enabled":
services.append(unit.rstrip(".timer"))
services.append(unit.replace(".timer", ""))
Copy link
Contributor

Choose a reason for hiding this comment

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

replace

Even better, use re.sub?

@@ -81,10 +81,10 @@ def mock_run_command_side_effect(*args, **kwargs):

if kwargs.get('return_cmd'):
if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
return 'snmp.timer'
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep original test case and add a new one?

@ganglyu ganglyu requested a review from qiluo-msft May 10, 2022 06:11
@ganglyu ganglyu merged commit 90abc07 into sonic-net:master May 12, 2022
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 25, 2022
Update sonic-utilities submodule pointer to include the following:
* [GCU] Handling type1 lists ([sonic-net#2171](sonic-net/sonic-utilities#2171))
* [yang] extend ConfigMgmt constructor to pass YANG options ([sonic-net#2118](sonic-net/sonic-utilities#2118))
* [dump] implement ACL modules ([sonic-net#2153](sonic-net/sonic-utilities#2153))
* show commands for SYSTEM READY ([sonic-net#1851](sonic-net/sonic-utilities#1851))
* [GCU] Handling non-compliant leaf-list with string values ([sonic-net#2174](sonic-net/sonic-utilities#2174))
* Add sonic-delayed.target to Application Extension .timer file generator ([sonic-net#2176](sonic-net/sonic-utilities#2176))
* [portconfig] Allow to configure interface mtu for physical ports ([#l](https://github.com/Azure/sonic-utilities/pull/l))
* Broadcast Unknown-multicast and Unknown-unicast Storm-control  ([sonic-net#928](sonic-net/sonic-utilities#928))
* sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071))
* [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel ([sonic-net#2151](sonic-net/sonic-utilities#2151))
* Modify override testcase to cover PORT admin_status ([sonic-net#2165](sonic-net/sonic-utilities#2165))
* [GCU] Validate peer_group_range ip_range are correct ([sonic-net#2145](sonic-net/sonic-utilities#2145))
* [auto-ts] add memory check ([sonic-net#2116](sonic-net/sonic-utilities#2116))
* support new interface types CR8/SR8/KR8/LR8 which are brougnt by SAI V.1.10.2 ([sonic-net#2167](sonic-net/sonic-utilities#2167))
* [scripts/fast-reboot] Add option to include ssd-upgrader-part boot option with SONiC partition ([sonic-net#2150](sonic-net/sonic-utilities#2150))
* [config reload] Fix invalid rstrip. ([sonic-net#2157](sonic-net/sonic-utilities#2157))
* Accept 0 for queue and dscp ([sonic-net#2162](sonic-net/sonic-utilities#2162))

Signed-off-by: dprital <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants