[KED-2768] Fix kedro install flow (#1203)

RELEASE.md

@@ -9,6 +9,7 @@
 
 ## Bug fixes and other changes
 * Bumped minimum required `fsspec` version to 2021.04.
+* Fixed the `kedro install` and `kedro build-reqs` flows when uninstalled dependencies are present in a project's `settings.py`, `context.py` or `hooks.py` ([Issue #829](https://github.com/quantumblacklabs/kedro/issues/829)).
 
 ## Minor breaking changes to the API
 

kedro/framework/cli/jupyter.py

@@ -54,6 +54,7 @@
     load_entry_points,
     python_call,
 )
+from kedro.framework.project import validate_settings
 from kedro.framework.startup import ProjectMetadata
 
 JUPYTER_IP_HELP = "IP address of the Jupyter server."
@@ -142,6 +143,8 @@ def jupyter_notebook(
     """Open Jupyter Notebook with project specific variables loaded."""
     _check_module_importable("jupyter_core")
 
+    validate_settings()
+
     if "-h" not in args and "--help" not in args:
         ipython_message(all_kernels)
 
@@ -179,6 +182,8 @@ def jupyter_lab(
     """Open Jupyter Lab with project specific variables loaded."""
     _check_module_importable("jupyter_core")
 
+    validate_settings()
+
     if "-h" not in args and "--help" not in args:
         ipython_message(all_kernels)
 

kedro/framework/project/__init__.py

@@ -203,20 +203,11 @@ def _clear(self, pipelines_module: str) -> None:
 pipelines = _ProjectPipelines()
 
 
-def _validate_module(settings_module):
-    """Eagerly validate that the module is importable.
-    This ensures that the settings module is syntactically
-    correct so that any import errors are surfaced early.
-    """
-    importlib.import_module(settings_module)
-
-
 def configure_project(package_name: str):
     """Configure a Kedro project by populating its settings with values
     defined in user's settings.py and pipeline_registry.py.
     """
     settings_module = f"{package_name}.settings"
-    _validate_module(settings_module)
     settings.configure(settings_module)
 
     # set up all hooks so we can discover all pipelines
@@ -228,7 +219,19 @@ def configure_project(package_name: str):
     pipelines.configure(pipelines_module)
 
     # Once the project is successfully configured once, store PACKAGE_NAME as a
-    # global variable to make it easily accessible. This is used by ParallelRunner on
-    # Windows, as package_name is required every time a new subprocess is spawned.
+    # global variable to make it easily accessible. This is used by validate_settings()
+    # below, and also by ParallelRunner on Windows, as package_name is required every
+    # time a new subprocess is spawned.
     global PACKAGE_NAME
     PACKAGE_NAME = package_name
+
+
+def validate_settings():
+    """Eagerly validate that the settings module is importable. This is desirable to
+    surface any syntax or import errors early. In particular, without eagerly importing
+    the settings module, dynaconf would silence any import error (e.g. missing
+    dependency, missing/mislabelled pipeline), and users would instead get a cryptic
+    error message ``Expected an instance of `ConfigLoader`, got `NoneType` instead``.
+    More info on the dynaconf issue: https://github.com/rochacbruno/dynaconf/issues/460
+    """
+    importlib.import_module(f"{PACKAGE_NAME}.settings")

kedro/framework/session/session.py

@@ -27,7 +27,6 @@
 # limitations under the License.
 # pylint: disable=invalid-name,global-statement
 """This module implements Kedro session responsible for project lifecycle."""
-
 import logging
 import logging.config
 import os
@@ -46,7 +45,12 @@
     _convert_paths_to_absolute_posix,
 )
 from kedro.framework.hooks import get_hook_manager
-from kedro.framework.project import configure_project, pipelines, settings
+from kedro.framework.project import (
+    configure_project,
+    pipelines,
+    settings,
+    validate_settings,
+)
 from kedro.framework.session.store import BaseSessionStore
 from kedro.io.core import generate_timestamp
 from kedro.runner import AbstractRunner, SequentialRunner
@@ -177,13 +181,15 @@ def create(  # pylint: disable=too-many-arguments
             A new ``KedroSession`` instance.
         """
 
-        # this is to make sure that for workflows that manually create session
-        # without going through one of our known entrypoints, e.g. some plugins like kedro-airflow,
-        # the project is still properly configured. This is for backward compatibility
-        # and should be removed in 0.18.
+        # This is to make sure that for workflows that manually create session
+        # without going through one of our known entrypoints, e.g. some plugins
+        # like kedro-airflow, the project is still properly configured. This
+        # is for backward compatibility and should be removed in 0.18.
         if package_name is not None:
             configure_project(package_name)
 
+        validate_settings()
+
         session = cls(
             package_name=package_name,
             project_path=project_path,

tests/extras/extensions/test_ipython.py

@@ -101,6 +101,7 @@ def test_load_kedro_objects(self, tmp_path, mocker):
             project_path=tmp_path,
         )
         mocker.patch("kedro.framework.session.session.configure_project")
+        mocker.patch("kedro.framework.session.session.validate_settings")
         mocker.patch(
             "kedro.framework.startup.bootstrap_project",
             return_value=fake_metadata,
@@ -125,7 +126,7 @@ def test_load_kedro_objects(self, tmp_path, mocker):
                 "session": mocker.ANY,
             }
         )
-        assert mock_register_line_magic.call_count == 1
+        mock_register_line_magic.assert_called_once()
 
     def test_load_kedro_objects_extra_args(self, tmp_path, mocker):
         fake_metadata = ProjectMetadata(

tests/framework/context/test_context.py

@@ -282,7 +282,6 @@ def mocked_logging(mocker):
 def dummy_context(
     tmp_path, prepare_project_dir, env, extra_params, mocker
 ):  # pylint: disable=unused-argument
-    mocker.patch("kedro.framework.project._validate_module")
     configure_project(MOCK_PACKAGE_NAME)
     context = KedroContext(
         MOCK_PACKAGE_NAME, str(tmp_path), env=env, extra_params=extra_params

tests/framework/project/test_pipeline_registry.py

@@ -78,17 +78,15 @@ def register_pipelines():
 
 
 def test_pipelines_after_configuring_project_shows_updated_values(
-    mock_package_name_with_pipelines_file, mocker
+    mock_package_name_with_pipelines_file,
 ):
-    mocker.patch("kedro.framework.project._validate_module")
     configure_project(mock_package_name_with_pipelines_file)
     assert isinstance(pipelines["new_pipeline"], Pipeline)
 
 
 def test_configure_project_should_not_raise_for_unimportable_pipelines(
-    mock_package_name_with_unimportable_pipelines_file, mocker
+    mock_package_name_with_unimportable_pipelines_file,
 ):
-    mocker.patch("kedro.framework.project._validate_module")
     # configure_project should not raise error for unimportable pipelines
     # since pipelines loading is lazy
     configure_project(mock_package_name_with_unimportable_pipelines_file)

tests/framework/session/conftest.py

@@ -25,6 +25,7 @@
 #
 # See the License for the specific language governing permissions and
 # limitations under the License.
+import importlib
 import logging
 from logging.handlers import QueueHandler, QueueListener
 from multiprocessing import Queue
@@ -50,10 +51,12 @@
 
 logger = logging.getLogger(__name__)
 
+MOCK_PACKAGE_NAME = "fake_package"
+
 
 @pytest.fixture
 def mock_package_name() -> str:
-    return "mock_package_name"
+    return MOCK_PACKAGE_NAME
 
 
 @pytest.fixture
@@ -402,14 +405,6 @@ def register_catalog(
         )
 
 
-@pytest.fixture(autouse=True)
-def patched_validate_module(mocker):
-    """Patching this so KedroSession could be created for testing purpose
-    since KedroSession.create is still calling configure_project at the moment
-    """
-    mocker.patch("kedro.framework.project._validate_module")
-
-
 @pytest.fixture
 def project_hooks():
     """A set of project hook implementations that log to stdout whenever it is invoked."""
@@ -464,3 +459,16 @@ def mock_session(
     return KedroSession.create(
         mock_package_name, tmp_path, extra_params={"params:key": "value"}
     )
+
+
+@pytest.fixture(autouse=True)
+def mock_import(mocker):
+    # KedroSession eagerly validates that a project's settings.py is correct by
+    # importing it. settings.py does not actually exists as part of this test suite,
+    # so its import is mocked.
+    settings_module = f"{MOCK_PACKAGE_NAME}.settings"
+    mocker.patch(
+        "importlib.import_module",
+        wraps=importlib.import_module,
+        side_effect=(lambda x: None if x == settings_module else mocker.DEFAULT),
+    )