code review suggestions

Author: leo-schick

mara_pipelines/pipelines.py

@@ -83,26 +83,26 @@ def html_doc_items(self) -> List[Tuple[str, str]]:
 class Task(Node):
     def __init__(self, id: str, description: str, commands: Optional[Union[Callable, List[Command]]] = None, max_retries: Optional[int] = None) -> None:
         super().__init__(id, description)
-        self.callable_commands = callable(commands)
+        self.is_dynamic_commands = callable(commands)
         self.max_retries = max_retries
 
-        if self.callable_commands:
+        if self.is_dynamic_commands:
             self._commands = None
-            self.__commands_callable = commands
+            self.__dynamic_commands_generator_func = commands
         else:
             self._commands = []
             self._add_commands(commands or [])
 
-    def _test_is_not_dynamic(self):
-        if self.callable_commands:
+    def _assert_is_not_dynamic(self):
+        if self.is_dynamic_commands:
             raise Exception('You cannot use add_command when the task is constructed with a callable commands function.')
 
     @property
     def commands(self) -> List:
-        if not self._commands:
+        if self._commands is None:
             self._commands = []
             # execute the callable command function and cache the result
-            for command in self.__commands_callable() or []:
+            for command in self.__dynamic_commands_generator_func() or []:
                 self._add_command(command)
         return self._commands
 
@@ -118,11 +118,11 @@ def _add_commands(self, commands: List[Command]):
             self._add_command(command)
 
     def add_command(self, command: Command, prepend=False):
-        self._test_is_not_dynamic()
+        self._assert_is_not_dynamic()
         self._add_command(command, prepend=prepend)
 
     def add_commands(self, commands: List[Command]):
-        self._test_is_not_dynamic()
+        self._assert_is_not_dynamic()
         self._add_commands(commands)
 
     def run(self):

mara_pipelines/ui/node_page.py

@@ -80,7 +80,7 @@ def __(pipeline: pipelines.Pipeline):
 def __(task: pipelines.Task):
     if not acl.current_user_has_permission(views.acl_resource):
         return bootstrap.card(header_left='Commands', body=acl.inline_permission_denied_message())
-    elif task.callable_commands:
+    elif task.is_dynamic_commands:
         return bootstrap.card(
             header_left='Commands',
             body=f"""<span style="font-style:italic;color:#aaa"><span class="fa fa-lightbulb"> </span> {"... are defined dynamically during execution"}</span>""")

tests/test_pipeline_tasks.py

@@ -31,7 +31,7 @@ def python_test_function(result: _PythonFuncTestResult):
     assert test_result.has_run
 
 
-def test_run_task_callable_commands():
+def test_run_task_dynamic_commands():
     """
     A simple test executing a task with callable commands
     """
@@ -48,8 +48,8 @@ def generate_command_list() -> List:
     assert not test_result.has_run
 
     task = Task(
-        id='run_task_callable_commands',
-        description="Unit test test_run_task_callable_commands",
+        id='run_task_dynamic_commands',
+        description="Unit test test_run_task_dynamic_commands",
         commands=generate_command_list)
 
     assert not test_result.has_run