From 570542dcb19803c404c6ca91564b97061d536ad0 Mon Sep 17 00:00:00 2001
From: Leonardo Giordani <giordani.leonardo@gmail.com>
Date: Sat, 21 Nov 2020 10:34:07 +0000
Subject: [PATCH] Improve logging of generators and writer loaders

This improves the logging of get_generator_classes() to
provide the source of the generator (either internal or from a plugin).
It also reviews the code to make it clearer (in my opinion, but I'm biased ;)), renames get_writer() to _get_writer_class() for consistency with the renamed _get_generator_classes().
---
 RELEASE.md                    |  3 ++
 pelican/__init__.py           | 82 ++++++++++++++++++++++-------------
 pelican/tests/test_pelican.py |  4 +-
 3 files changed, 56 insertions(+), 33 deletions(-)
 create mode 100644 RELEASE.md

diff --git a/RELEASE.md b/RELEASE.md
new file mode 100644
index 00000000..384c99f0
--- /dev/null
+++ b/RELEASE.md
@@ -0,0 +1,3 @@
+Release type: patch
+
+Improve logging of generators and writer loaders
diff --git a/pelican/__init__.py b/pelican/__init__.py
index 82366117..237005b3 100644
--- a/pelican/__init__.py
+++ b/pelican/__init__.py
@@ -93,7 +93,7 @@ class Pelican:
                 path=self.path,
                 theme=self.theme,
                 output_path=self.output_path,
-            ) for cls in self.get_generator_classes()
+            ) for cls in self._get_generator_classes()
         ]
 
         # Delete the output directory if (1) the appropriate setting is True
@@ -114,7 +114,7 @@ class Pelican:
 
         signals.all_generators_finalized.send(generators)
 
-        writer = self.get_writer()
+        writer = self._get_writer_class()
 
         for p in generators:
             if hasattr(p, 'generate_output'):
@@ -162,46 +162,66 @@ class Pelican:
                     pluralized_draft_pages,
                     time.time() - start_time))
 
-    def get_generator_classes(self):
-        generators = [ArticlesGenerator, PagesGenerator]
+    def _get_generator_classes(self):
+        discovered_generators = [
+            (ArticlesGenerator, None),
+            (PagesGenerator, None)
+        ]
 
-        if self.settings['TEMPLATE_PAGES']:
-            generators.append(TemplatePagesGenerator)
-        if self.settings['OUTPUT_SOURCES']:
-            generators.append(SourceFileGenerator)
+        if self.settings["TEMPLATE_PAGES"]:
+            discovered_generators.append((TemplatePagesGenerator, None))
 
-        for pair in signals.get_generators.send(self):
-            (funct, value) = pair
+        if self.settings["OUTPUT_SOURCES"]:
+            discovered_generators.append((SourceFileGenerator, None))
 
-            if not isinstance(value, Iterable):
-                value = (value, )
+        for receiver, values in signals.get_generators.send(self):
+            if not isinstance(values, Iterable):
+                values = (values,)
 
-            for v in value:
-                if isinstance(v, type):
-                    logger.debug('Found generator: %s', v)
-                    generators.append(v)
+                discovered_generators.extend(
+                    [(generator, receiver) for generator in values]
+                )
 
         # StaticGenerator must run last, so it can identify files that
         # were skipped by the other generators, and so static files can
         # have their output paths overridden by the {attach} link syntax.
-        generators.append(StaticGenerator)
+        discovered_generators.append((StaticGenerator, None))
+
+        generators = []
+
+        logger.debug("Loading generators:")
+
+        for generator, receiver in discovered_generators:
+            if receiver is None:
+                origin = "internal"
+            else:
+                origin = f"{receiver.__module__}"
+
+            if not isinstance(generator, type):
+                logger.error(f"Generator {generator} ({origin}) cannot be loaded")
+                continue
+
+            logger.debug(f"* {generator.__name__} ({origin})")
+            generators.append(generator)
+
         return generators
 
-    def get_writer(self):
-        writers = [w for (_, w) in signals.get_writer.send(self)
-                   if isinstance(w, type)]
-        writers_found = len(writers)
-        if writers_found == 0:
+    def _get_writer_class(self):
+        writers = [w for _, w in signals.get_writer.send(self) if isinstance(w, type)]
+        num_writers = len(writers)
+
+        if num_writers == 0:
             return Writer(self.output_path, settings=self.settings)
-        else:
-            writer = writers[0]
-            if writers_found == 1:
-                logger.debug('Found writer: %s', writer)
-            else:
-                logger.warning(
-                    '%s writers found, using only first one: %s',
-                    writers_found, writer)
-            return writer(self.output_path, settings=self.settings)
+
+        if num_writers > 1:
+            logger.warning(
+                f"{num_writers} writers found, using only first one"
+            )
+
+        writer = writers[0]
+
+        logger.debug(f"Found writer: {writer}")
+        return writer(self.output_path, settings=self.settings)
 
 
 class PrintSettings(argparse.Action):
diff --git a/pelican/tests/test_pelican.py b/pelican/tests/test_pelican.py
index 219377ae..cacc65c1 100644
--- a/pelican/tests/test_pelican.py
+++ b/pelican/tests/test_pelican.py
@@ -84,14 +84,14 @@ class TestPelican(LoggedTestCase):
         # have their output paths overridden by the {attach} link syntax.
 
         pelican = Pelican(settings=read_settings(path=None))
-        generator_classes = pelican.get_generator_classes()
+        generator_classes = pelican._get_generator_classes()
 
         self.assertTrue(
             generator_classes[-1] is StaticGenerator,
             "StaticGenerator must be the last generator, but it isn't!")
         self.assertIsInstance(
             generator_classes, Sequence,
-            "get_generator_classes() must return a Sequence to preserve order")
+            "_get_generator_classes() must return a Sequence to preserve order")
 
     def test_basic_generation_works(self):
         # when running pelican without settings, it should pick up the default