Add argparse custom action refactoring comment

joe4dev · joe4dev · commit 4301369969b9 · 2023-03-20T14:54:14.000+01:00
Requires more testing to ensure consistent behavior (e.g., with multiple arguments, empty arguments, and combinations).
diff --git a/localstack/utils/container_utils/container_client.py b/localstack/utils/container_utils/container_client.py
@@ -956,6 +956,9 @@ def parse_additional_flags(
         :return: A DockerRunFlags object that will return new objects if respective parameters were None and
                 additional flags contained a flag for that object or the same which are passed otherwise.
         """
+        # Argparse refactoring opportunity: custom argparse actions can be used to modularize parsing (e.g., key=value)
+        # https://docs.python.org/3/library/argparse.html#action
+
         # Configure parser
         parser = NoExitArgumentParser(description="Docker run flags parser")
         parser.add_argument(
diff --git a/localstack/utils/no_exit_argument_parser.py b/localstack/utils/no_exit_argument_parser.py
@@ -5,11 +5,13 @@
 LOG = logging.getLogger(__name__)
 
 
-# Implements the `exit_on_error=False` behavior introduced in Python 3.9 to support older Python versions
-# and prevents further SystemExit for other error categories.
-# Limitations of error cases: https://stackoverflow.com/a/67891066/6875981
-# Subclassing workaround example: https://stackoverflow.com/a/59072378/6875981
 class NoExitArgumentParser(argparse.ArgumentParser):
+    """Implements the `exit_on_error=False` behavior introduced in Python 3.9 to support older Python versions
+    and prevents further SystemExit for other error categories.
+    * Limitations of error categories: https://stackoverflow.com/a/67891066/6875981
+    * ArgumentParser subclassing example: https://stackoverflow.com/a/59072378/6875981
+    """
+
     def exit(self, status: int = ..., message: Optional[str] = ...) -> NoReturn:
         LOG.warning(f"Error in argument parser but preventing exit: {message}")
 
diff --git a/tests/unit/test_dockerclient.py b/tests/unit/test_dockerclient.py
@@ -168,6 +168,9 @@ def test_labels(self):
         argument_string = r'--label ""'  # assert that we gracefully handle invalid labels
         flags = Util.parse_additional_flags(argument_string)
         assert flags.labels == {}
+        argument_string = r"--label =bar"  # assert that we ignore empty labels
+        flags = Util.parse_additional_flags(argument_string)
+        assert flags.labels == {}
 
     def test_network(self):
         argument_string = r'-v "/tmp/test.jar:/tmp/foo bar/test.jar" --network mynet123'
