Fix type bug in build_trackers -- skip non-callable modules (#1249)

Summary:

Fix Pyre type error in `build_trackers()` where `load_module()` can return
a `ModuleType` that is then called as a factory function.

**Problem:**
- `load_module(factory_name)` returns `ModuleType | Callable | None`
- When the user passes a bare module path (no `:attr` suffix), a `ModuleType`
  is returned, which is not callable
- Calling `factory(config)` on a `ModuleType` is a runtime `TypeError` and
  a Pyre type error [29]

**Fix:**
- Add a `callable()` check after resolving the factory
- Non-callable modules are skipped with a descriptive warning guiding the user
  to use `module.path:factory_function` syntax
- Rename intermediate variable to `resolved` so Pyre can narrow the type
  before assigning to `factory`

**Cleanup:**
- Convert f-string log calls to `%s` formatting per TorchX conventions

Differential Revision: D95932184

Author: kiukchung

torchx/tracker/api.py

@@ -155,16 +155,26 @@ def build_trackers(
         logger.warning("No 'torchx.tracker' entry_points are defined.")
 
     for factory_name, config in factory_and_config.items():
-        factory = entrypoint_factories.get(factory_name) or load_module(factory_name)
-        if not factory:
+        resolved = entrypoint_factories.get(factory_name) or load_module(factory_name)
+        if not resolved:
             logger.warning(
-                f"No tracker factory `{factory_name}` found in entry_points or modules. See https://meta-pytorch.org/torchx/main/tracker.html#module-torchx.tracker"
+                "no tracker factory `%s` found in entry_points or modules."
+                " See https://meta-pytorch.org/torchx/main/tracker.html#module-torchx.tracker",
+                factory_name,
             )
             continue
+        if not callable(resolved):
+            logger.warning(
+                "tracker factory `%s` resolved to a module, not a callable."
+                " Use `module.path:factory_function` syntax to specify the factory",
+                factory_name,
+            )
+            continue
+        factory = resolved
         if config:
-            logger.info(f"Tracker config found for `{factory_name}` as `{config}`")
+            logger.info("tracker config found for `%s` as `%s`", factory_name, config)
         else:
-            logger.info(f"No tracker config specified for `{factory_name}`")
+            logger.info("no tracker config specified for `%s`", factory_name)
         tracker = factory(config)
         trackers.append(tracker)
     return trackers

torchx/tracker/test/api_test.py

@@ -83,8 +83,6 @@ def sources(
         run_id: str,
         artifact_name: str | None = None,
     ) -> Iterable[TrackerSource]:
-        source_data = self._sources[run_id]
-
         sources = []
         for artifact_name, source_id in self._sources[run_id].items():
             if artifact_name == DEFAULT_SOURCE:
@@ -289,6 +287,28 @@ def test_build_trackers_with_module(self) -> None:
             self.assertIsInstance(tracker, MLflowTracker)
             module.assert_called_once_with(config)
 
+    def test_build_trackers_with_non_callable_module(self) -> None:
+        """load_module returns a ModuleType when no :attr is specified.
+        Modules aren't callable -- build_trackers should skip them with a warning.
+        """
+        from types import ModuleType
+
+        module = ModuleType("fake_tracker_module")
+        with (
+            patch("torchx.tracker.api.load_group", return_value=None),
+            patch(
+                "torchx.tracker.api.load_module",
+                return_value=module,
+            ),
+        ):
+            tracker_names = {"fake_tracker_module": "myconfig.txt"}
+            trackers = build_trackers(tracker_names)
+            self.assertEqual(
+                0,
+                len(list(trackers)),
+                "non-callable module should be skipped, not called as a factory",
+            )
+
     def test_build_trackers(self) -> None:
         with patch(
             "torchx.tracker.api.load_group",