Skip to content

Commit

Permalink
Fix dangerous use of default arguments in python function call.
Browse files Browse the repository at this point in the history
You cannot safely use = [] as a default argument in
Python; subsequent calls will reuse that object.  It probably
doesn't matter in this case, as these are all "read-only"
arguments, but it is better not to do it at all.  Instead,
we recognize that this function is always called with
all of its arguments, so there is no need for default arguments
at all.

While in here, I noticed that, generate_artifacts was returning
a boolean on an error path, which is not allowed by its signature.  So fix
that as well.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
  • Loading branch information
clalancette committed May 22, 2024
1 parent fb22661 commit b1739fd
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions sros2/sros2/api/_artifact_generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,15 @@
from . import _policy


# FIXME move away from mutable default (linter should complain about it)
def generate_artifacts(
keystore_path: Optional[pathlib.Path] = None,
identity_names: List[str] = [],
policy_files: List[pathlib.Path] = []
keystore_path: Optional[pathlib.Path],
identity_names: List[str],
policy_files: List[pathlib.Path]
) -> None:
if keystore_path is None:
keystore_path = _utilities.get_keystore_path_from_env()
if keystore_path is None:
return False
return

Check warning on line 32 in sros2/sros2/api/_artifact_generation.py

View check run for this annotation

Codecov / codecov/patch

sros2/sros2/api/_artifact_generation.py#L32

Added line #L32 was not covered by tests
if not keystore.is_valid_keystore(keystore_path):
print('%s is not a valid keystore, creating new keystore' % keystore_path)
keystore.create_keystore(keystore_path)
Expand Down

0 comments on commit b1739fd

Please sign in to comment.