Merge branch 'master' into feature/run-method-validation

Author: awaelchli

src/lightning_app/CHANGELOG.md

@@ -10,12 +10,16 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 ### Added
 
 - Add `load_state_dict` and `state_dict` ([#14100](https://github.com/Lightning-AI/lightning/pull/14100))
+
+
 - Add `--secret` option to CLI to allow binding Secrets to app environment variables when running in the cloud ([#14612](https://github.com/Lightning-AI/lightning/pull/14612))
 
 
 ### Changed
 
 - Application storage prefix moved from `app_id` to `project_id/app_id` ([#14583](https://github.com/Lightning-AI/lightning/pull/14583))
+
+
 - LightningCloud client calls to use key word arguments instead of positional arguments ([#14685](https://github.com/Lightning-AI/lightning/pull/14685)
 
 
@@ -25,22 +29,29 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Improved the error messsage when the `LightningWork` is missing the `run` method ([#14759](https://github.com/Lightning-AI/lightning/pull/14759))
 
 
+- Improved the error messsage when the root `LightningFlow` passed to `LightningApp` is missing the `run` method ([#14760](https://github.com/Lightning-AI/lightning/pull/14760))
+
+
+
 
 ### Fixed
 
 - Making threadpool non default from LightningCloud client  ([#14757](https://github.com/Lightning-AI/lightning/pull/14757))
 
+
 - Resolved a bug where the state change detection using DeepDiff won't worked with Path, Drive objects ([#14465](https://github.com/Lightning-AI/lightning/pull/14465))
 
+
 - Resolved a bug where the wrong client was passed to collect cloud logs ([#14684](https://github.com/Lightning-AI/lightning/pull/14684))
 
+
 - Resolved the memory leak issue with Lightning Cloud package and bumped the requirements to use the latest version ([#14697](https://github.com/Lightning-AI/lightning/pull/14697)
 
 
 - Unification of app template: moved `app.py` to root dir for `lightning init app <app_name>` template ([#13853](https://github.com/Lightning-AI/lightning/pull/13853))
 
-- Fixing 5000 log line limitation for Lightning AI BYOC cluster logs ([#14458](https://github.com/Lightning-AI/lightning/pull/14458))
 
+- Fixing 5000 log line limitation for Lightning AI BYOC cluster logs ([#14458](https://github.com/Lightning-AI/lightning/pull/14458))
 
 
 - Fixed a bug where the uploaded command file wasn't properly parsed ([#14532](https://github.com/Lightning-AI/lightning/pull/14532))
@@ -49,6 +60,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Resolved `LightningApp(..., debug=True)` ([#14464](https://github.com/Lightning-AI/lightning/pull/14464))
 
 
+- Fixed an issue where custom property setters were not being used `LightningWork` class. ([#14259](https://github.com/Lightning-AI/lightning/pull/14259))
+
+
 ## [0.6.0] - 2022-09-08
 
 ### Added

src/lightning_app/core/app.py

@@ -26,7 +26,7 @@
 from lightning_app.storage.path import storage_root_dir
 from lightning_app.utilities.app_helpers import _delta_to_app_state_delta, _LightningAppRef, Logger
 from lightning_app.utilities.commands.base import _process_requests
-from lightning_app.utilities.component import _convert_paths_after_init
+from lightning_app.utilities.component import _convert_paths_after_init, _validate_root_flow
 from lightning_app.utilities.enum import AppStage, CacheCallsKeys
 from lightning_app.utilities.exceptions import CacheMissException, ExitAppException
 from lightning_app.utilities.layout import _collect_layout
@@ -58,8 +58,8 @@ def __init__(
         the :class:`~lightning.app.core.flow.LightningFlow` provided.
 
         Arguments:
-            root: The root LightningFlow component, that defined all
-                the app's nested components, running infinitely.
+            root: The root LightningFlow component, that defines all the app's nested components, running infinitely.
+                It must define a `run()` method that the app can call.
             debug: Whether to activate the Lightning Logger debug mode.
                 This can be helpful when reporting bugs on Lightning repo.
 
@@ -77,6 +77,7 @@ def __init__(
             Hello World!
         """
 
+        _validate_root_flow(root)
         self._root = root
 
         # queues definition.

src/lightning_app/core/work.py

@@ -2,7 +2,7 @@
 import warnings
 from copy import deepcopy
 from functools import partial, wraps
-from typing import Any, Callable, Dict, List, Optional
+from typing import Any, Callable, Dict, List, Optional, Union
 
 from deepdiff import DeepHash
 
@@ -316,9 +316,17 @@ def num_successes(self) -> int:
 
         return has_succeeded_counter
 
+    def _get_property_if_exists(self, name: str) -> Union[property, None]:
+        attr = getattr(self.__class__, name, None)
+        return attr if isinstance(attr, property) else None
+
     def __setattr__(self, name: str, value: Any) -> None:
-        setattr_fn = getattr(self, "_setattr_replacement", None) or self._default_setattr
-        setattr_fn(name, value)
+        property_object = self._get_property_if_exists(name)
+        if property_object is not None and property_object.fset is not None:
+            property_object.fset(self, value)
+        else:
+            setattr_fn = getattr(self, "_setattr_replacement", None) or self._default_setattr
+            setattr_fn(name, value)
 
     def _default_setattr(self, name: str, value: Any) -> None:
         from lightning_app.core.flow import LightningFlow

src/lightning_app/utilities/component.py

@@ -5,6 +5,7 @@
 from deepdiff.helper import NotPresent
 from lightning_utilities.core.apply_func import apply_to_collection
 
+from lightning_app.utilities.app_helpers import is_overridden
 from lightning_app.utilities.enum import ComponentContext
 from lightning_app.utilities.tree import breadth_first
 
@@ -118,3 +119,13 @@ def _context(ctx: str) -> Generator[None, None, None]:
     _set_context(ctx)
     yield
     _set_context(prev)
+
+
+def _validate_root_flow(flow: "LightningFlow") -> None:
+    from lightning_app.core.flow import LightningFlow
+
+    if not is_overridden("run", instance=flow, parent=LightningFlow):
+        raise TypeError(
+            "The root flow passed to `LightningApp` does not override the `run()` method. This is required. Please"
+            f" implement `run()` in your `{flow.__class__.__name__}` class."
+        )

tests/tests_app/core/test_lightning_app.py

@@ -1,6 +1,7 @@
 import logging
 import os
 import pickle
+from re import escape
 from time import sleep
 from unittest import mock
 from unittest.mock import ANY
@@ -32,6 +33,29 @@
 logger = logging.getLogger()
 
 
+def test_lightning_app_requires_root_run_method():
+    """Test that a useful exception is raised if the root flow does not override the run method."""
+
+    with pytest.raises(
+        TypeError, match=escape("The root flow passed to `LightningApp` does not override the `run()` method")
+    ):
+        LightningApp(LightningFlow())
+
+    class FlowWithoutRun(LightningFlow):
+        pass
+
+    with pytest.raises(
+        TypeError, match=escape("The root flow passed to `LightningApp` does not override the `run()` method")
+    ):
+        LightningApp(FlowWithoutRun())
+
+    class FlowWithRun(LightningFlow):
+        def run(self):
+            pass
+
+    LightningApp(FlowWithRun())  # no error
+
+
 class B1(LightningFlow):
     def __init__(self):
         super().__init__()

tests/tests_app/core/test_lightning_work.py

@@ -6,7 +6,7 @@
 
 from lightning_app import LightningApp
 from lightning_app.core.flow import LightningFlow
-from lightning_app.core.work import LightningWork, LightningWorkException
+from lightning_app.core.work import BuildConfig, LightningWork, LightningWorkException
 from lightning_app.runners import MultiProcessRuntime
 from lightning_app.storage import Path
 from lightning_app.testing.helpers import EmptyFlow, EmptyWork, MockQueue
@@ -293,3 +293,37 @@ def run(self, *args, **kwargs):
     w.run(1, [2], (3, 4), {"1": "3"})
     assert len(w._calls) == 2
     assert w._calls["0d824f7"] == {"ret": None}
+
+
+def test_work_cloud_build_config_provided():
+
+    assert isinstance(LightningWork.cloud_build_config, property)
+    assert LightningWork.cloud_build_config.fset is not None
+
+    class Work(LightningWork):
+        def __init__(self):
+            super().__init__()
+            self.cloud_build_config = BuildConfig(image="ghcr.io/gridai/base-images:v1.8-cpu")
+
+        def run(self, *args, **kwargs):
+            pass
+
+    w = Work()
+    w.run()
+
+
+def test_work_local_build_config_provided():
+
+    assert isinstance(LightningWork.local_build_config, property)
+    assert LightningWork.local_build_config.fset is not None
+
+    class Work(LightningWork):
+        def __init__(self):
+            super().__init__()
+            self.local_build_config = BuildConfig(image="ghcr.io/gridai/base-images:v1.8-cpu")
+
+        def run(self, *args, **kwargs):
+            pass
+
+    w = Work()
+    w.run()