fix: Do not pass display as param since it's a singleton + init local…

…e to fix warning (#101)

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 1.1.0 (unreleased)
+
+- fix: Do not pass display as param since it's a singleton + init locale to fix warning
+
 # 1.0.2 (2022-01-16)
 
 * fix: Fix include_role with loop by @haidaraM in https://github.com/haidaraM/ansible-playbook-grapher/pull/92

ansibleplaybookgrapher/cli.py

@@ -20,7 +20,7 @@
 from ansible.cli import CLI
 from ansible.cli.arguments import option_helpers
 from ansible.release import __version__ as ansible_version
-from ansible.utils.display import Display
+from ansible.utils.display import Display, initialize_locale
 
 from ansibleplaybookgrapher import __prog__, __version__
 from ansibleplaybookgrapher.parser import PlaybookParser
@@ -48,14 +48,16 @@ def run(self):
         # The display is a singleton. This instruction will NOT return a new instance.
         # We explicitly set the verbosity after the init.
         display = Display()
+        # Required to fix the warning "ansible.utils.display.initialize_locale has not been called..."
+        initialize_locale()
         display.verbosity = self.options.verbosity
 
-        parser = PlaybookParser(display=display, tags=self.options.tags, skip_tags=self.options.skip_tags,
+        parser = PlaybookParser(tags=self.options.tags, skip_tags=self.options.skip_tags,
                                 playbook_filename=self.options.playbook_filename,
                                 include_role_tasks=self.options.include_role_tasks)
 
         playbook_node = parser.parse()
-        renderer = GraphvizRenderer(playbook_node, display)
+        renderer = GraphvizRenderer(playbook_node)
         svg_path = renderer.render(self.options.output_filename, self.options.save_dot_file, self.options.view)
 
         post_processor = GraphVizPostProcessor(svg_path=svg_path)

ansibleplaybookgrapher/parser.py

@@ -31,18 +31,19 @@
 from ansibleplaybookgrapher.utils import clean_name, handle_include_path, has_role_parent, generate_id, \
     convert_when_to_str
 
+display = Display()
+
 
 class BaseParser(ABC):
     """
     Base Parser of a playbook
     """
 
-    def __init__(self, tags: List[str] = None, skip_tags: List[str] = None, display: Display = None):
+    def __init__(self, tags: List[str] = None, skip_tags: List[str] = None):
         """
 
         :param tags: Only add plays and tasks tagged with these values
         :param skip_tags: Only add plays and tasks whose tags do not match these values
-        :param display: Ansible display used to print some messages in the console
         """
         loader, inventory, variable_manager = CLI._play_prereqs()
         self.data_loader = loader
@@ -51,7 +52,6 @@ def __init__(self, tags: List[str] = None, skip_tags: List[str] = None, display:
 
         self.tags = tags or ["all"]
         self.skip_tags = skip_tags or []
-        self.display = display or Display()
 
     @abstractmethod
     def parse(self, *args, **kwargs) -> PlaybookNode:
@@ -73,7 +73,7 @@ def template(self, data: Union[str, AnsibleUnicode], variables: Dict,
             # Sometimes we need to export
             if fail_on_undefined:
                 raise
-            self.display.warning(ansible_error)
+            display.warning(ansible_error)
             return data
 
     def _add_task(self, task: Task, task_vars: Dict, node_type: str, parent_node: CompositeNode) -> bool:
@@ -87,10 +87,10 @@ def _add_task(self, task: Task, task_vars: Dict, node_type: str, parent_node: Co
             return False
 
         if not task.evaluate_tags(only_tags=self.tags, skip_tags=self.skip_tags, all_vars=task_vars):
-            self.display.vv(f"The task '{task.get_name()}' is skipped due to the tags.")
+            display.vv(f"The task '{task.get_name()}' is skipped due to the tags.")
             return False
 
-        self.display.vv(f"Adding {node_type} '{task.get_name()}' to the graph")
+        display.vv(f"Adding {node_type} '{task.get_name()}' to the graph")
 
         task_name = clean_name(self.template(task.get_name(), task_vars))
         edge_label = convert_when_to_str(task.when)
@@ -108,16 +108,15 @@ class PlaybookParser(BaseParser):
     """
 
     def __init__(self, playbook_filename: str, include_role_tasks=False, tags: List[str] = None,
-                 skip_tags: List[str] = None, display: Display = None):
+                 skip_tags: List[str] = None):
         """
         :param playbook_filename: The filename of the playbook to parse
-        :param display: Ansible display used to print some messages in the console
         :param include_role_tasks: If true, the tasks of the role will be included in the graph
         :param tags: Only add plays and tasks tagged with these values
         :param skip_tags: Only add plays and tasks whose tags do not match these values
         """
 
-        super().__init__(tags=tags, skip_tags=skip_tags, display=display)
+        super().__init__(tags=tags, skip_tags=skip_tags)
 
         self.include_role_tasks = include_role_tasks
         self.playbook_filename = playbook_filename
@@ -150,26 +149,26 @@ def parse(self, *args, **kwargs) -> PlaybookNode:
                 self.data_loader.set_basedir(play._included_path)
             else:
                 self.data_loader.set_basedir(self.playbook._basedir)
-            self.display.vvv(f"Loader basedir set to {self.data_loader.get_basedir()}")
+            display.vvv(f"Loader basedir set to {self.data_loader.get_basedir()}")
 
             play_vars = self.variable_manager.get_vars(play)
             play_hosts = [h.get_name() for h in self.inventory_manager.get_hosts(self.template(play.hosts, play_vars))]
             play_name = f"Play: {clean_name(play.get_name())} ({len(play_hosts)})"
             play_name = self.template(play_name, play_vars)
 
-            self.display.banner("Parsing " + play_name)
+            display.banner("Parsing " + play_name)
 
             play_node = PlayNode(play_name, hosts=play_hosts, raw_object=play)
             self.playbook_root_node.add_play(play_node, "")
 
             # loop through the pre_tasks
-            self.display.v("Parsing pre_tasks...")
+            display.v("Parsing pre_tasks...")
             for pre_task_block in play.pre_tasks:
                 self._include_tasks_in_blocks(current_play=play, parent_nodes=[play_node], block=pre_task_block,
                                               play_vars=play_vars, node_type="pre_task")
 
             # loop through the roles
-            self.display.v("Parsing roles...")
+            display.v("Parsing roles...")
 
             for role in play.get_roles():
                 # Don't insert tasks from ``import/include_role``, preventing duplicate graphing
@@ -179,7 +178,7 @@ def parse(self, *args, **kwargs) -> PlaybookNode:
                 # the role object doesn't inherit the tags from the play. So we add it manually.
                 role.tags = role.tags + play.tags
                 if not role.evaluate_tags(only_tags=self.tags, skip_tags=self.skip_tags, all_vars=play_vars):
-                    self.display.vv(f"The role '{role.get_name()}' is skipped due to the tags.")
+                    display.vv(f"The role '{role.get_name()}' is skipped due to the tags.")
                     # Go to the next role
                     continue
 
@@ -195,25 +194,25 @@ def parse(self, *args, **kwargs) -> PlaybookNode:
                 # end of roles loop
 
             # loop through the tasks
-            self.display.v("Parsing tasks...")
+            display.v("Parsing tasks...")
             for task_block in play.tasks:
                 self._include_tasks_in_blocks(current_play=play, parent_nodes=[play_node], block=task_block,
                                               play_vars=play_vars, node_type="task")
 
             # loop through the post_tasks
-            self.display.v("Parsing post_tasks...")
+            display.v("Parsing post_tasks...")
             for post_task_block in play.post_tasks:
                 self._include_tasks_in_blocks(current_play=play, parent_nodes=[play_node], block=post_task_block,
                                               play_vars=play_vars, node_type="post_task")
             # Summary
-            self.display.display("")  # just an empty line
-            self.display.v(f"{len(play_node.pre_tasks)} pre_task(s) added to the graph.")
-            self.display.v(f"{len(play_node.roles)} role(s) added to the play")
-            self.display.v(f"{len(play_node.tasks)} task(s) added to the play")
-            self.display.v(f"{len(play_node.post_tasks)} post_task(s) added to the play")
-
-            self.display.banner(f"Done parsing {play_name}")
-            self.display.display("")  # just an empty line
+            display.display("")  # just an empty line
+            display.v(f"{len(play_node.pre_tasks)} pre_task(s) added to the graph.")
+            display.v(f"{len(play_node.roles)} role(s) added to the play")
+            display.v(f"{len(play_node.tasks)} task(s) added to the play")
+            display.v(f"{len(play_node.post_tasks)} post_task(s) added to the play")
+
+            display.banner(f"Done parsing {play_name}")
+            display.display("")  # just an empty line
             # moving to the next play
 
         return self.playbook_root_node
@@ -242,8 +241,8 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
         for task_or_block in block.block:
 
             if hasattr(task_or_block, "loop") and task_or_block.loop:
-                self.display.warning("Looping on tasks or roles are not supported for the moment. "
-                                     f"Only the task having the loop argument will be added to the graph.")
+                display.warning("Looping on tasks or roles are not supported for the moment. "
+                                f"Only the task having the loop argument will be added to the graph.")
 
             if isinstance(task_or_block, Block):
                 self._include_tasks_in_blocks(current_play=current_play, parent_nodes=parent_nodes, block=task_or_block,
@@ -257,7 +256,7 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
                     # Here we have an 'include_role'. The class IncludeRole is a subclass of TaskInclude.
                     # We do this because the management of an 'include_role' is different.
                     # See :func:`~ansible.playbook.included_file.IncludedFile.process_include_results` from line 155
-                    self.display.v(f"An 'include_role' found. Including tasks from '{task_or_block.get_name()}'")
+                    display.v(f"An 'include_role' found. Including tasks from '{task_or_block.get_name()}'")
 
                     role_node = RoleNode(task_or_block.get_name(), raw_object=task_or_block)
                     parent_nodes[-1].add_node(f"{node_type}s", EdgeNode(parent_nodes[-1], role_node,
@@ -274,15 +273,15 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
                         block_list, _ = task_or_block.get_block_list(play=current_play, loader=self.data_loader,
                                                                      variable_manager=self.variable_manager)
                 else:
-                    self.display.v(f"An 'include_tasks' found. Including tasks from '{task_or_block.get_name()}'")
+                    display.v(f"An 'include_tasks' found. Including tasks from '{task_or_block.get_name()}'")
 
                     templar = Templar(loader=self.data_loader, variables=task_vars)
                     try:
                         included_file_path = handle_include_path(original_task=task_or_block, loader=self.data_loader,
                                                                  templar=templar)
                     except AnsibleUndefinedVariable as e:
                         # TODO: mark this task with some special shape or color
-                        self.display.warning(
+                        display.warning(
                             f"Unable to translate the include task '{task_or_block.get_name()}' due to an undefined variable: {str(e)}. "
                             "Some variables are available only during the execution of the playbook.")
                         self._add_task(task=task_or_block, task_vars=task_vars, node_type=node_type,
@@ -291,7 +290,7 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
 
                     data = self.data_loader.load_from_file(included_file_path)
                     if data is None:
-                        self.display.warning(f"The file '{included_file_path}' is empty and has no tasks to include")
+                        display.warning(f"The file '{included_file_path}' is empty and has no tasks to include")
                         continue
                     elif not isinstance(data, list):
                         raise AnsibleParserError("Included task files must contain a list of tasks", obj=data)
@@ -323,7 +322,7 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
                 # check if this task comes from a role, and we don't want to include tasks of the role
                 if has_role_parent(task_or_block) and not self.include_role_tasks:
                     # skip role's task
-                    self.display.vv(
+                    display.vv(
                         f"The task '{task_or_block.get_name()}' has a role as parent and include_role_tasks is false. "
                         "It will be skipped.")
                     # skipping

ansibleplaybookgrapher/renderer.py

@@ -21,6 +21,8 @@
 from ansibleplaybookgrapher.graph import PlaybookNode, EdgeNode, PlayNode, RoleNode, BlockNode
 from ansibleplaybookgrapher.utils import get_play_colors
 
+display = Display()
+
 
 class GraphvizRenderer:
     """
@@ -29,17 +31,15 @@ class GraphvizRenderer:
     DEFAULT_EDGE_ATTR = {"sep": "10", "esep": "5"}
     DEFAULT_GRAPH_ATTR = {"ratio": "fill", "rankdir": "LR", "concentrate": "true", "ordering": "in"}
 
-    def __init__(self, playbook_node: 'PlaybookNode', display: Display, graph_format: str = "svg",
+    def __init__(self, playbook_node: 'PlaybookNode', graph_format: str = "svg",
                  graph_attr: Dict = None, edge_attr: Dict = None):
         """
 
         :param playbook_node: Playbook parsed node
-        :param display: Display
         :param graph_format: the graph format to render. See https://graphviz.org/docs/outputs/
         :param graph_attr: Default graph attributes
         :param edge_attr: Default edge attributes
         """
-        self.display = display
         self.playbook_node = playbook_node
         self.digraph = Digraph(format=graph_format,
                                graph_attr=graph_attr or GraphvizRenderer.DEFAULT_GRAPH_ATTR,
@@ -129,7 +129,7 @@ def _convert_to_graphviz(self):
         Convert the PlaybookNode to the graphviz dot format
         :return:
         """
-        self.display.vvv(f"Converting the graph to the dot format for graphviz")
+        display.vvv(f"Converting the graph to the dot format for graphviz")
         # root node
         self.digraph.node(self.playbook_node.name, style="dotted", id="root_node")
 
@@ -180,14 +180,14 @@ def render(self, output_filename: str, save_dot_file=False, view=False) -> str:
         """
         self._convert_to_graphviz()
 
-        self.display.display("Rendering the graph...")
+        display.display("Rendering the graph...")
         rendered_file_path = self.digraph.render(cleanup=not save_dot_file, format="svg", filename=output_filename,
                                                  view=view)
 
         if save_dot_file:
             # add .dot extension. The render doesn't add an extension
             final_name = output_filename + ".dot"
             os.rename(output_filename, final_name)
-            self.display.display(f"Graphviz dot file has been exported to {final_name}")
+            display.display(f"Graphviz dot file has been exported to {final_name}")
 
         return rendered_file_path

tests/conftest.py

@@ -37,7 +37,7 @@ def fixture_variable_manager(data_loader, inventory_manager):
     return VariableManager(loader=data_loader, inventory=inventory_manager)
 
 
-@pytest.fixture(scope="session")
+@pytest.fixture(scope="session", autouse=True)
 def display():
     """
     Return a display

tests/test_parser.py

@@ -30,7 +30,7 @@ def test_include_role_parsing(grapher_cli: PlaybookGrapherCLI, display: Display,
     :param display:
     :return:
     """
-    parser = PlaybookParser(grapher_cli.options.playbook_filename, display=display, include_role_tasks=True)
+    parser = PlaybookParser(grapher_cli.options.playbook_filename, include_role_tasks=True)
     playbook_node = parser.parse()
     assert len(playbook_node.plays) == 1
     play_node = playbook_node.plays[0].destination
@@ -78,7 +78,7 @@ def test_block_parsing(grapher_cli: PlaybookGrapherCLI, display: Display):
     :param display:
     :return:
     """
-    parser = PlaybookParser(grapher_cli.options.playbook_filename, display=display, include_role_tasks=True)
+    parser = PlaybookParser(grapher_cli.options.playbook_filename, include_role_tasks=True)
     playbook_node = parser.parse()
     assert len(playbook_node.plays) == 1