Skip to content

Conversation

l0crian1
Copy link
Contributor

@l0crian1 l0crian1 commented Sep 6, 2025

Change summary

I've often found myself needing to return a default value when using dict_search, requiring me to use .get() instead, which makes for inconsistent code for ultimately the same function.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T7740

Related PR(s)

How to test / Smoketest result

Python Test script:

from vyos.utils.dict import dict_search

test_data = {
    "interfaces": {
        "ethernet": {
            "eth0": {
                "address": "192.0.2.1/24",
                "description": "WAN uplink"
            },
            "eth1": {
                "address": "10.0.0.1/24",
                "description": "LAN"
            }
        }
    },
    "system": {
        "hostname": "vyos-router"
    }
}

print(dict_search("interfaces.ethernet.eth0.address", test_data))       # 192.0.2.1/24
print(dict_search("interfaces.ethernet.eth1.description", test_data))   # LAN
print(dict_search("interfaces.ethernet.eth3", test_data))               # None
print(dict_search("system.hostname", test_data))                        # vyos-router
print(dict_search("system.login", test_data, "not found"))      # not found

Results:

root@vyos:/home/vyos# python3 test.py
192.0.2.1/24
LAN
None
vyos-router
not found

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

- Update dict_search to return optional default value
Copy link

github-actions bot commented Sep 6, 2025

👍
No issues in PR Title / Commit Title

@jestabro jestabro requested review from dmbaturin, jestabro, c-po and sarthurdev and removed request for dmbaturin September 11, 2025 13:30
Copy link
Member

@sarthurdev sarthurdev left a comment

Choose a reason for hiding this comment

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

Useful extension to dict_search

@c-po c-po added the bp/circinus Create automatic backport for circinus label Sep 14, 2025
Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

I've often found myself needing to return a default value when using dict_search, requiring me to use .get() instead, which makes for inconsistent code for ultimately the same function.

I found myself in the same situation but was to biased to find such an easy solution. Thanks! As you already outlined some testcases - please be so kind and extend the build time test suite to cover these as well everytime. Those are run by make test or make all during DEB package assembly.

https://github.com/vyos/vyos-1x/blob/current/src/tests/test_dict_search.py

Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

Requested change addressed

@c-po c-po merged commit f406d48 into vyos:current Sep 14, 2025
8 of 9 checks passed
@vyosbot vyosbot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Sep 14, 2025
Copy link

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests VPP 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • Config tests VPP 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bp/circinus Create automatic backport for circinus current mirror-completed rebase
Development

Successfully merging this pull request may close these issues.

4 participants