Skip to content

config: T0000: fix mutable default argument in config API methods #5075

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.

Sign up for GitHub

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

Draft
andamasov wants to merge 1 commit into
base: current
Choose a base branch
from
Draft

config: T0000: fix mutable default argument in config API methods #5075

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions python/vyos/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ def return_value(self, path, default=None):
else:
return(value)

def return_values(self, path, default=[]):
def return_values(self, path, default=None):
"""
Comment on lines +483 to 484
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The PR description says mutable defaults were fixed for the Config API methods listed, but this file still contains mutable default arguments (e.g., get_config_dict(self, path=...) and get_config_defaults(self, path=...) use path=[]). Either include those in this change set or adjust the PR description/scope so it’s not misleading.

Copilot uses AI. Check for mistakes.
Retrieve all values of a multi-value leaf node in the running or proposed config

Expand All @@ -495,6 +495,8 @@ def return_values(self, path, default=[]):
This function cannot be used outside a configuration session.
In operational mode scripts, use ``return_effective_values``.
"""
if default is None:
default = []
if self._session_config:
try:
values = self._session_config.return_values(self._make_path(path))
Expand All @@ -508,7 +510,7 @@ def return_values(self, path, default=[]):
else:
return(values)

def list_nodes(self, path, default=[]):
def list_nodes(self, path, default=None):
"""
Retrieve names of all children of a tag node in the running or proposed config

Expand All @@ -519,6 +521,8 @@ def list_nodes(self, path, default=[]):
string list: child node names

"""
Comment on lines 513 to 523
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

default is accepted by this method but not described in the docstring. Please add it to Args and clarify what is returned when the path doesn't exist (currently default.copy() when no nodes are found).

Copilot uses AI. Check for mistakes.
if default is None:
default = []
if self._session_config:
try:
nodes = self._session_config.list_nodes(self._make_path(path))
Expand Down Expand Up @@ -596,7 +600,7 @@ def return_effective_value(self, path, default=None):
else:
return(value)

def return_effective_values(self, path, default=[]):
def return_effective_values(self, path, default=None):
"""
Retrieve all values of a multi-value node in a running (effective) config

Expand All @@ -606,6 +610,8 @@ def return_effective_values(self, path, default=[]):
Returns:
str list: A list of values
"""
Comment on lines 603 to 612
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

This docstring doesn't mention the default parameter even though it's part of the signature, and it doesn't explain the fallback behavior when the node is missing/empty. Please document default and the exact return semantics.

Copilot uses AI. Check for mistakes.
if default is None:
default = []
if self._running_config:
try:
values = self._running_config.return_values(self._make_path(path))
Expand All @@ -619,7 +625,7 @@ def return_effective_values(self, path, default=[]):
else:
return(values)

def list_effective_nodes(self, path, default=[]):
def list_effective_nodes(self, path, default=None):
"""
Retrieve names of all children of a tag node in the running config

Expand All @@ -629,6 +635,8 @@ def list_effective_nodes(self, path, default=[]):
Returns:
str list: child node names
"""
Comment on lines 628 to 637
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

default is part of the signature but isn't documented. Please add it to the docstring and clarify that the method returns default.copy() when no nodes are found.

Copilot uses AI. Check for mistakes.
if default is None:
default = []
if self._running_config:
try:
nodes = self._running_config.list_nodes(self._make_path(path))
Expand Down
Loading