From 19c0b70340f7559745ed8c41b6003f182cbbcc78 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 15:41:19 +0100 Subject: [PATCH 01/22] Package update from watchgod to watchfiles Signed-off-by: Jitendra Gundaniya --- package/features/steps/lower_requirements.txt | 2 +- package/kedro_viz/launchers/cli/run.py | 14 +++++++++----- package/kedro_viz/launchers/jupyter.py | 14 +++++++++----- package/kedro_viz/server.py | 14 +++++++++----- package/requirements.txt | 2 +- package/tests/test_launchers/test_cli/test_run.py | 14 ++++++++------ 6 files changed, 37 insertions(+), 23 deletions(-) diff --git a/package/features/steps/lower_requirements.txt b/package/features/steps/lower_requirements.txt index 1a149e7ad9..6e12855128 100644 --- a/package/features/steps/lower_requirements.txt +++ b/package/features/steps/lower_requirements.txt @@ -3,7 +3,7 @@ fastapi==0.100.0 fsspec==2021.4 aiofiles==22.1.0 uvicorn[standard]==0.22.0 -watchgod==0.8.2 +watchfiles==0.24.0 plotly==4.8 packaging==23.0 pandas==1.3; python_version < '3.10' diff --git a/package/kedro_viz/launchers/cli/run.py b/package/kedro_viz/launchers/cli/run.py index 97c9ab3dbc..8a2117faa9 100644 --- a/package/kedro_viz/launchers/cli/run.py +++ b/package/kedro_viz/launchers/cli/run.py @@ -12,6 +12,8 @@ _VIZ_PROCESSES: Dict[str, int] = {} +def custom_filter(_, path: str) -> bool: + return path.endswith(('.yml', '.yaml', '.py', '.json')) @viz.command(context_settings={"help_option_names": ["-h", "--help"]}) @click.option( @@ -164,17 +166,19 @@ def run( "is_lite": lite, } if autoreload: - from watchgod import RegExpWatcher, run_process + from watchfiles import run_process + run_process_args = [str(kedro_project_path)] run_process_kwargs = { - "path": kedro_project_path, "target": run_server, "kwargs": run_server_kwargs, - "watcher_cls": RegExpWatcher, - "watcher_kwargs": {"re_files": r"^.*(\.yml|\.yaml|\.py|\.json)$"}, + "watch_filter": custom_filter, } viz_process = multiprocessing.Process( - target=run_process, daemon=False, kwargs={**run_process_kwargs} + target=run_process, + daemon=False, + args=run_process_args, + kwargs={**run_process_kwargs} ) else: viz_process = multiprocessing.Process( diff --git a/package/kedro_viz/launchers/jupyter.py b/package/kedro_viz/launchers/jupyter.py index f51f6ce7eb..01066ec3fb 100644 --- a/package/kedro_viz/launchers/jupyter.py +++ b/package/kedro_viz/launchers/jupyter.py @@ -14,7 +14,7 @@ import IPython from IPython.display import HTML, display from kedro.framework.project import PACKAGE_NAME -from watchgod import RegExpWatcher, run_process +from watchfiles import run_process from kedro_viz.launchers.utils import _check_viz_up, _wait_for from kedro_viz.server import DEFAULT_HOST, DEFAULT_PORT, run_server @@ -24,6 +24,8 @@ logger = logging.getLogger(__name__) +def custom_filter(_, file_path: str) -> bool: + return file_path.endswith((".yml", ".yaml", ".py", ".json")) def _allocate_port(host: str, start_at: int, end_at: int = 65535) -> int: acceptable_ports = range(start_at, end_at + 1) @@ -148,15 +150,17 @@ def run_viz( # pylint: disable=too-many-locals } process_context = multiprocessing.get_context("spawn") if autoreload: + run_process_args = [str(project_path)] run_process_kwargs = { - "path": project_path, "target": run_server, "kwargs": run_server_kwargs, - "watcher_cls": RegExpWatcher, - "watcher_kwargs": {"re_files": r"^.*(\.yml|\.yaml|\.py|\.json)$"}, + "watch_filter": custom_filter, } viz_process = process_context.Process( - target=run_process, daemon=False, kwargs={**run_process_kwargs} + target=run_process, + daemon=False, + args=run_process_args, + kwargs={**run_process_kwargs} ) else: viz_process = process_context.Process( diff --git a/package/kedro_viz/server.py b/package/kedro_viz/server.py index 37d31f2f19..c46fb06bb2 100644 --- a/package/kedro_viz/server.py +++ b/package/kedro_viz/server.py @@ -70,6 +70,8 @@ def load_and_populate_data( # Creates data repositories which are used by Kedro Viz Backend APIs populate_data(data_access_manager, catalog, pipelines, session_store, stats_dict) +def custom_filter(_, file_path: str) -> bool: + return file_path.endswith(('.yml', '.yaml', '.py', '.json')) # pylint: disable=too-many-positional-arguments, too-many-locals def run_server( @@ -142,7 +144,7 @@ def run_server( import argparse import multiprocessing - from watchgod import RegExpWatcher, run_process + from watchfiles import run_process parser = argparse.ArgumentParser(description="Launch a development viz server") parser.add_argument("project_path", help="Path to a Kedro project") @@ -156,20 +158,22 @@ def run_server( project_path = (Path.cwd() / args.project_path).absolute() + run_process_args = [str(project_path)] run_process_kwargs = { - "path": project_path, "target": run_server, "kwargs": { "host": args.host, "port": args.port, "project_path": str(project_path), }, - "watcher_cls": RegExpWatcher, - "watcher_kwargs": {"re_files": r"^.*(\.yml|\.yaml|\.py|\.json)$"}, + "watch_filter": custom_filter, } viz_process = multiprocessing.Process( - target=run_process, daemon=False, kwargs={**run_process_kwargs} + target=run_process, + daemon=False, + args=run_process_args, + kwargs={**run_process_kwargs} ) print("Starting Kedro Viz ...") diff --git a/package/requirements.txt b/package/requirements.txt index caf3fa63ea..f16575bf6a 100644 --- a/package/requirements.txt +++ b/package/requirements.txt @@ -15,4 +15,4 @@ secure>=0.3.0 sqlalchemy>=1.4, <3 strawberry-graphql>=0.192.0, <1.0 uvicorn[standard]>=0.30.0, <1.0 -watchgod>=0.8.2, <1.0 +watchfiles>=0.24.0 diff --git a/package/tests/test_launchers/test_cli/test_run.py b/package/tests/test_launchers/test_cli/test_run.py index b2d5c59b39..0df9d630b3 100644 --- a/package/tests/test_launchers/test_cli/test_run.py +++ b/package/tests/test_launchers/test_cli/test_run.py @@ -4,11 +4,11 @@ import requests from click.testing import CliRunner from packaging.version import parse -from watchgod import RegExpWatcher, run_process +from watchfiles import run_process from kedro_viz import __version__ from kedro_viz.launchers.cli import main -from kedro_viz.launchers.cli.run import _VIZ_PROCESSES +from kedro_viz.launchers.cli.run import _VIZ_PROCESSES, custom_filter from kedro_viz.launchers.utils import _PYPROJECT from kedro_viz.server import run_server @@ -357,8 +357,8 @@ def test_kedro_viz_command_with_autoreload( with runner.isolated_filesystem(): runner.invoke(main.viz_cli, ["viz", "run", "--autoreload"]) + run_process_args = [str(mock_project_path)] run_process_kwargs = { - "path": mock_project_path, "target": run_server, "kwargs": { "host": "127.0.0.1", @@ -374,11 +374,13 @@ def test_kedro_viz_command_with_autoreload( "extra_params": {}, "is_lite": False, }, - "watcher_cls": RegExpWatcher, - "watcher_kwargs": {"re_files": "^.*(\\.yml|\\.yaml|\\.py|\\.json)$"}, + "watch_filter": custom_filter } process_init.assert_called_once_with( - target=run_process, daemon=False, kwargs={**run_process_kwargs} + target=run_process, + daemon=False, + args=run_process_args, + kwargs={**run_process_kwargs} ) assert run_process_kwargs["kwargs"]["port"] in _VIZ_PROCESSES From 9bace9ef708e7726146ca7f47ce31067b720ea6e Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 15:48:20 +0100 Subject: [PATCH 02/22] lint fix Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/launchers/cli/run.py | 10 ++++++---- package/kedro_viz/launchers/jupyter.py | 8 +++++--- package/kedro_viz/server.py | 10 ++++++---- package/tests/test_launchers/test_cli/test_run.py | 8 ++++---- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/package/kedro_viz/launchers/cli/run.py b/package/kedro_viz/launchers/cli/run.py index 8a2117faa9..d48ff6f069 100644 --- a/package/kedro_viz/launchers/cli/run.py +++ b/package/kedro_viz/launchers/cli/run.py @@ -12,8 +12,10 @@ _VIZ_PROCESSES: Dict[str, int] = {} + def custom_filter(_, path: str) -> bool: - return path.endswith(('.yml', '.yaml', '.py', '.json')) + return path.endswith((".yml", ".yaml", ".py", ".json")) + @viz.command(context_settings={"help_option_names": ["-h", "--help"]}) @click.option( @@ -175,10 +177,10 @@ def run( "watch_filter": custom_filter, } viz_process = multiprocessing.Process( - target=run_process, - daemon=False, + target=run_process, + daemon=False, args=run_process_args, - kwargs={**run_process_kwargs} + kwargs={**run_process_kwargs}, ) else: viz_process = multiprocessing.Process( diff --git a/package/kedro_viz/launchers/jupyter.py b/package/kedro_viz/launchers/jupyter.py index 01066ec3fb..35c63ffda7 100644 --- a/package/kedro_viz/launchers/jupyter.py +++ b/package/kedro_viz/launchers/jupyter.py @@ -24,9 +24,11 @@ logger = logging.getLogger(__name__) + def custom_filter(_, file_path: str) -> bool: return file_path.endswith((".yml", ".yaml", ".py", ".json")) + def _allocate_port(host: str, start_at: int, end_at: int = 65535) -> int: acceptable_ports = range(start_at, end_at + 1) @@ -157,10 +159,10 @@ def run_viz( # pylint: disable=too-many-locals "watch_filter": custom_filter, } viz_process = process_context.Process( - target=run_process, - daemon=False, + target=run_process, + daemon=False, args=run_process_args, - kwargs={**run_process_kwargs} + kwargs={**run_process_kwargs}, ) else: viz_process = process_context.Process( diff --git a/package/kedro_viz/server.py b/package/kedro_viz/server.py index c46fb06bb2..1753a54513 100644 --- a/package/kedro_viz/server.py +++ b/package/kedro_viz/server.py @@ -70,8 +70,10 @@ def load_and_populate_data( # Creates data repositories which are used by Kedro Viz Backend APIs populate_data(data_access_manager, catalog, pipelines, session_store, stats_dict) + def custom_filter(_, file_path: str) -> bool: - return file_path.endswith(('.yml', '.yaml', '.py', '.json')) + return file_path.endswith((".yml", ".yaml", ".py", ".json")) + # pylint: disable=too-many-positional-arguments, too-many-locals def run_server( @@ -170,10 +172,10 @@ def run_server( } viz_process = multiprocessing.Process( - target=run_process, - daemon=False, + target=run_process, + daemon=False, args=run_process_args, - kwargs={**run_process_kwargs} + kwargs={**run_process_kwargs}, ) print("Starting Kedro Viz ...") diff --git a/package/tests/test_launchers/test_cli/test_run.py b/package/tests/test_launchers/test_cli/test_run.py index 0df9d630b3..f135db8828 100644 --- a/package/tests/test_launchers/test_cli/test_run.py +++ b/package/tests/test_launchers/test_cli/test_run.py @@ -374,13 +374,13 @@ def test_kedro_viz_command_with_autoreload( "extra_params": {}, "is_lite": False, }, - "watch_filter": custom_filter + "watch_filter": custom_filter, } process_init.assert_called_once_with( - target=run_process, - daemon=False, + target=run_process, + daemon=False, args=run_process_args, - kwargs={**run_process_kwargs} + kwargs={**run_process_kwargs}, ) assert run_process_kwargs["kwargs"]["port"] in _VIZ_PROCESSES From 69a699cdfaa499c8bd28ab8a196e5b8324bb5a50 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 15:58:33 +0100 Subject: [PATCH 03/22] Test fix Signed-off-by: Jitendra Gundaniya --- package/tests/test_launchers/test_jupyter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/package/tests/test_launchers/test_jupyter.py b/package/tests/test_launchers/test_jupyter.py index dd489778ca..485e7ff890 100644 --- a/package/tests/test_launchers/test_jupyter.py +++ b/package/tests/test_launchers/test_jupyter.py @@ -140,6 +140,7 @@ def test_run_viz_with_autoreload(self, mocker, patched_check_viz_up): mock_process.assert_called_once_with( target=mocker.ANY, daemon=False, # No daemon for autoreload + args=mocker.ANY, kwargs=mocker.ANY, ) From 1363650117e38733f423c54410b2a3b31944c2be Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 16:27:34 +0100 Subject: [PATCH 04/22] Test fixes Signed-off-by: Jitendra Gundaniya --- .../kedro_viz/data_access/repositories/catalog.py | 2 +- package/kedro_viz/launchers/cli/run.py | 7 ++----- package/kedro_viz/launchers/jupyter.py | 7 ++----- package/kedro_viz/server.py | 7 ++----- package/kedro_viz/utils.py | 14 ++++++++++++++ package/tests/test_launchers/test_cli/test_run.py | 5 +++-- 6 files changed, 24 insertions(+), 18 deletions(-) diff --git a/package/kedro_viz/data_access/repositories/catalog.py b/package/kedro_viz/data_access/repositories/catalog.py index a7d568ca8a..38d9a6772d 100644 --- a/package/kedro_viz/data_access/repositories/catalog.py +++ b/package/kedro_viz/data_access/repositories/catalog.py @@ -131,7 +131,7 @@ def get_dataset(self, dataset_name: str) -> Optional["AbstractDataset"]: else: # pragma: no cover dataset_obj = self._catalog._get_dataset(dataset_name) except DatasetNotFoundError: - dataset_obj = MemoryDataset() # pylint: disable=abstract-class-instantiated + dataset_obj = MemoryDataset() return dataset_obj diff --git a/package/kedro_viz/launchers/cli/run.py b/package/kedro_viz/launchers/cli/run.py index d48ff6f069..35cd1d043f 100644 --- a/package/kedro_viz/launchers/cli/run.py +++ b/package/kedro_viz/launchers/cli/run.py @@ -9,14 +9,11 @@ from kedro_viz.constants import DEFAULT_HOST, DEFAULT_PORT from kedro_viz.launchers.cli.main import viz +from kedro_viz.utils import file_extension_filter _VIZ_PROCESSES: Dict[str, int] = {} -def custom_filter(_, path: str) -> bool: - return path.endswith((".yml", ".yaml", ".py", ".json")) - - @viz.command(context_settings={"help_option_names": ["-h", "--help"]}) @click.option( "--host", @@ -174,7 +171,7 @@ def run( run_process_kwargs = { "target": run_server, "kwargs": run_server_kwargs, - "watch_filter": custom_filter, + "watch_filter": file_extension_filter, } viz_process = multiprocessing.Process( target=run_process, diff --git a/package/kedro_viz/launchers/jupyter.py b/package/kedro_viz/launchers/jupyter.py index 35c63ffda7..0e434b9d94 100644 --- a/package/kedro_viz/launchers/jupyter.py +++ b/package/kedro_viz/launchers/jupyter.py @@ -18,6 +18,7 @@ from kedro_viz.launchers.utils import _check_viz_up, _wait_for from kedro_viz.server import DEFAULT_HOST, DEFAULT_PORT, run_server +from kedro_viz.utils import file_extension_filter _VIZ_PROCESSES: Dict[str, int] = {} _DATABRICKS_HOST = "0.0.0.0" @@ -25,10 +26,6 @@ logger = logging.getLogger(__name__) -def custom_filter(_, file_path: str) -> bool: - return file_path.endswith((".yml", ".yaml", ".py", ".json")) - - def _allocate_port(host: str, start_at: int, end_at: int = 65535) -> int: acceptable_ports = range(start_at, end_at + 1) @@ -156,7 +153,7 @@ def run_viz( # pylint: disable=too-many-locals run_process_kwargs = { "target": run_server, "kwargs": run_server_kwargs, - "watch_filter": custom_filter, + "watch_filter": file_extension_filter, } viz_process = process_context.Process( target=run_process, diff --git a/package/kedro_viz/server.py b/package/kedro_viz/server.py index 1753a54513..5ab7aa36b2 100644 --- a/package/kedro_viz/server.py +++ b/package/kedro_viz/server.py @@ -15,6 +15,7 @@ from kedro_viz.integrations.kedro import data_loader as kedro_data_loader from kedro_viz.integrations.kedro.sqlite_store import SQLiteStore from kedro_viz.launchers.utils import _check_viz_up, _wait_for +from kedro_viz.utils import file_extension_filter DEV_PORT = 4142 @@ -71,10 +72,6 @@ def load_and_populate_data( populate_data(data_access_manager, catalog, pipelines, session_store, stats_dict) -def custom_filter(_, file_path: str) -> bool: - return file_path.endswith((".yml", ".yaml", ".py", ".json")) - - # pylint: disable=too-many-positional-arguments, too-many-locals def run_server( host: str = DEFAULT_HOST, @@ -168,7 +165,7 @@ def run_server( "port": args.port, "project_path": str(project_path), }, - "watch_filter": custom_filter, + "watch_filter": file_extension_filter, } viz_process = multiprocessing.Process( diff --git a/package/kedro_viz/utils.py b/package/kedro_viz/utils.py index a0a4a5abce..bd24cb0a53 100644 --- a/package/kedro_viz/utils.py +++ b/package/kedro_viz/utils.py @@ -57,3 +57,17 @@ def _strip_transcoding(element: str) -> str: def is_dataset_param(dataset_name: str) -> bool: """Return whether a dataset is a parameter""" return dataset_name.lower().startswith("params:") or dataset_name == "parameters" + + +def file_extension_filter(_, file_path: str) -> bool: + """ + Determine if a given file path ends with one of the specified extensions. + + Args: + _: Unused parameter. + file_path (str): The path of the file to check. + + Returns: + bool: True if the file path ends with '.yml', '.yaml', '.py', or '.json', False otherwise. + """ + return file_path.endswith((".yml", ".yaml", ".py", ".json")) diff --git a/package/tests/test_launchers/test_cli/test_run.py b/package/tests/test_launchers/test_cli/test_run.py index f135db8828..c358647bb3 100644 --- a/package/tests/test_launchers/test_cli/test_run.py +++ b/package/tests/test_launchers/test_cli/test_run.py @@ -8,9 +8,10 @@ from kedro_viz import __version__ from kedro_viz.launchers.cli import main -from kedro_viz.launchers.cli.run import _VIZ_PROCESSES, custom_filter +from kedro_viz.launchers.cli.run import _VIZ_PROCESSES from kedro_viz.launchers.utils import _PYPROJECT from kedro_viz.server import run_server +from kedro_viz.utils import file_extension_filter @pytest.fixture @@ -374,7 +375,7 @@ def test_kedro_viz_command_with_autoreload( "extra_params": {}, "is_lite": False, }, - "watch_filter": custom_filter, + "watch_filter": file_extension_filter, } process_init.assert_called_once_with( From f6f4bc390431221271e40022c2b453aca34acc1f Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 16:36:37 +0100 Subject: [PATCH 05/22] lint fixes Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/package/kedro_viz/utils.py b/package/kedro_viz/utils.py index bd24cb0a53..bb47a30372 100644 --- a/package/kedro_viz/utils.py +++ b/package/kedro_viz/utils.py @@ -64,7 +64,6 @@ def file_extension_filter(_, file_path: str) -> bool: Determine if a given file path ends with one of the specified extensions. Args: - _: Unused parameter. file_path (str): The path of the file to check. Returns: From 810be7883dc74089e5a4d568dcf4f648e733bca5 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 16:52:32 +0100 Subject: [PATCH 06/22] test fix Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/utils.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/package/kedro_viz/utils.py b/package/kedro_viz/utils.py index bb47a30372..fb9fd0e9f3 100644 --- a/package/kedro_viz/utils.py +++ b/package/kedro_viz/utils.py @@ -60,13 +60,5 @@ def is_dataset_param(dataset_name: str) -> bool: def file_extension_filter(_, file_path: str) -> bool: - """ - Determine if a given file path ends with one of the specified extensions. - - Args: - file_path (str): The path of the file to check. - - Returns: - bool: True if the file path ends with '.yml', '.yaml', '.py', or '.json', False otherwise. - """ + """Determine if a given file path ends with one of the specified extensions.""" return file_path.endswith((".yml", ".yaml", ".py", ".json")) From 59480d0ffc5a1675f51814c9d62b08d4863db8fc Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 19:52:16 +0100 Subject: [PATCH 07/22] test added Signed-off-by: Jitendra Gundaniya --- package/tests/test_utils.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 package/tests/test_utils.py diff --git a/package/tests/test_utils.py b/package/tests/test_utils.py new file mode 100644 index 0000000000..10490dab74 --- /dev/null +++ b/package/tests/test_utils.py @@ -0,0 +1,17 @@ +import pytest +from kedro_viz.utils import file_extension_filter + +@pytest.mark.parametrize( + "file_path, expected", + [ + ("config.yml", True), + ("config.yaml", True), + ("script.py", True), + ("data.json", True), + ("image.png", False), + ("document.txt", False), + ("archive.zip", False), + ], +) +def test_file_extension_filter(file_path, expected): + assert file_extension_filter(None, file_path) == expected \ No newline at end of file From 589faf81929b7e9dce3395a1a3770f51b6645fd9 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 19:54:59 +0100 Subject: [PATCH 08/22] lint fix Signed-off-by: Jitendra Gundaniya --- package/tests/test_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package/tests/test_utils.py b/package/tests/test_utils.py index 10490dab74..414976ed05 100644 --- a/package/tests/test_utils.py +++ b/package/tests/test_utils.py @@ -1,6 +1,8 @@ import pytest + from kedro_viz.utils import file_extension_filter + @pytest.mark.parametrize( "file_path, expected", [ @@ -14,4 +16,4 @@ ], ) def test_file_extension_filter(file_path, expected): - assert file_extension_filter(None, file_path) == expected \ No newline at end of file + assert file_extension_filter(None, file_path) == expected From 28649b6829c6cfbc7e4c77c27666b5086f4d5627 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 9 Oct 2024 20:39:00 +0100 Subject: [PATCH 09/22] Release note added Signed-off-by: Jitendra Gundaniya --- RELEASE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE.md b/RELEASE.md index b93010273a..82da35fc12 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -15,6 +15,7 @@ Please follow the established format: ## Bug fixes and other changes - Fix unserializable parameters value (#2122) +- Replace `watchgod` library with `watchfiles` (#2134) # Release 10.0.0 From 6e1659fbb4f5bd965f9e66d007a137731a9a5936 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Tue, 22 Oct 2024 15:01:33 +0100 Subject: [PATCH 10/22] AutoreloadFileFilter added for accurate auto reload files Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/autoreload_file_filter.py | 79 +++++++++++++ package/kedro_viz/launchers/cli/run.py | 10 +- package/kedro_viz/launchers/jupyter.py | 4 +- package/kedro_viz/server.py | 8 +- package/kedro_viz/utils.py | 5 - package/tests/test_autoreload_file_filter.py | 116 +++++++++++++++++++ 6 files changed, 208 insertions(+), 14 deletions(-) create mode 100644 package/kedro_viz/autoreload_file_filter.py create mode 100644 package/tests/test_autoreload_file_filter.py diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py new file mode 100644 index 0000000000..8d407f49f0 --- /dev/null +++ b/package/kedro_viz/autoreload_file_filter.py @@ -0,0 +1,79 @@ +import logging +from pathlib import Path +from typing import Optional, Set + +import pathspec +from watchfiles import Change, DefaultFilter + +logger = logging.getLogger(__name__) + + +class AutoreloadFileFilter(DefaultFilter): + """ + Custom file filter for autoreloading that extends DefaultFilter. + Filters out files based on allowed file extensions and patterns specified in a .gitignore file. + """ + + allowed_extensions: Set[str] = {".py", ".yml", ".yaml", ".json"} + + def __init__(self, base_path: Optional[Path] = None): + """ + Initialize the AutoreloadFileFilter. + Args: + base_path (Optional[Path]): The base path to set as the current working directory for the filter. + """ + self.cwd = base_path or Path.cwd() + + # Call the superclass constructor + super().__init__() + + # Load .gitignore patterns + gitignore_path = self.cwd / ".gitignore" + try: + with open(gitignore_path, "r") as gitignore_file: + ignore_patterns = gitignore_file.read().splitlines() + self.gitignore_spec = pathspec.PathSpec.from_lines( + "gitwildmatch", ignore_patterns + ) + except FileNotFoundError: + self.gitignore_spec = None + + def __call__(self, change: Change, path: str) -> bool: + """ + Determine whether a file change should be processed. + Args: + change (Change): The type of change detected. + path (str): The path to the file that changed. + Returns: + bool: True if the file should be processed, False otherwise. + """ + if not super().__call__(change, path): + logger.debug(f"Filtered out by DefaultFilter: {path}") + return False + + path_obj = Path(path) + + # Exclude files matching .gitignore patterns + try: + relative_path = path_obj.resolve().relative_to(self.cwd.resolve()) + except ValueError: + logger.debug(f"Path not relative to CWD: {path}") + return False + + try: + if self.gitignore_spec and self.gitignore_spec.match_file( + str(relative_path) + ): + logger.debug(f"Filtered out by .gitignore: {relative_path}") + return False + except Exception as e: + logger.debug(f"Exception during .gitignore matching: {e}") + return True # Pass the file if .gitignore matching fails + + # Include only files with allowed extensions + if path_obj.suffix in self.allowed_extensions: + logger.debug(f"Allowed file: {path}") + return True + else: + logger.debug(f"Filtered out by allowed_extensions: {path_obj.suffix}") + return False diff --git a/package/kedro_viz/launchers/cli/run.py b/package/kedro_viz/launchers/cli/run.py index 35cd1d043f..703ca1505a 100644 --- a/package/kedro_viz/launchers/cli/run.py +++ b/package/kedro_viz/launchers/cli/run.py @@ -7,9 +7,9 @@ from kedro.framework.cli.project import PARAMS_ARG_HELP from kedro.framework.cli.utils import _split_params +from kedro_viz.autoreload_file_filter import AutoreloadFileFilter from kedro_viz.constants import DEFAULT_HOST, DEFAULT_PORT from kedro_viz.launchers.cli.main import viz -from kedro_viz.utils import file_extension_filter _VIZ_PROCESSES: Dict[str, int] = {} @@ -164,6 +164,8 @@ def run( "extra_params": params, "is_lite": lite, } + + process_context = multiprocessing.get_context("spawn") if autoreload: from watchfiles import run_process @@ -171,16 +173,16 @@ def run( run_process_kwargs = { "target": run_server, "kwargs": run_server_kwargs, - "watch_filter": file_extension_filter, + "watch_filter": AutoreloadFileFilter(), } - viz_process = multiprocessing.Process( + viz_process = process_context.Process( target=run_process, daemon=False, args=run_process_args, kwargs={**run_process_kwargs}, ) else: - viz_process = multiprocessing.Process( + viz_process = process_context.Process( target=run_server, daemon=False, kwargs={**run_server_kwargs} ) diff --git a/package/kedro_viz/launchers/jupyter.py b/package/kedro_viz/launchers/jupyter.py index 0e434b9d94..d7bf10a228 100644 --- a/package/kedro_viz/launchers/jupyter.py +++ b/package/kedro_viz/launchers/jupyter.py @@ -16,9 +16,9 @@ from kedro.framework.project import PACKAGE_NAME from watchfiles import run_process +from kedro_viz.autoreload_file_filter import AutoreloadFileFilter from kedro_viz.launchers.utils import _check_viz_up, _wait_for from kedro_viz.server import DEFAULT_HOST, DEFAULT_PORT, run_server -from kedro_viz.utils import file_extension_filter _VIZ_PROCESSES: Dict[str, int] = {} _DATABRICKS_HOST = "0.0.0.0" @@ -153,7 +153,7 @@ def run_viz( # pylint: disable=too-many-locals run_process_kwargs = { "target": run_server, "kwargs": run_server_kwargs, - "watch_filter": file_extension_filter, + "watch_filter": AutoreloadFileFilter(), } viz_process = process_context.Process( target=run_process, diff --git a/package/kedro_viz/server.py b/package/kedro_viz/server.py index 5ab7aa36b2..225524076e 100644 --- a/package/kedro_viz/server.py +++ b/package/kedro_viz/server.py @@ -9,13 +9,13 @@ from kedro.pipeline import Pipeline from kedro_viz.api.rest.responses import save_api_responses_to_fs +from kedro_viz.autoreload_file_filter import AutoreloadFileFilter from kedro_viz.constants import DEFAULT_HOST, DEFAULT_PORT from kedro_viz.data_access import DataAccessManager, data_access_manager from kedro_viz.database import make_db_session_factory from kedro_viz.integrations.kedro import data_loader as kedro_data_loader from kedro_viz.integrations.kedro.sqlite_store import SQLiteStore from kedro_viz.launchers.utils import _check_viz_up, _wait_for -from kedro_viz.utils import file_extension_filter DEV_PORT = 4142 @@ -165,10 +165,12 @@ def run_server( "port": args.port, "project_path": str(project_path), }, - "watch_filter": file_extension_filter, + "watch_filter": AutoreloadFileFilter(), } - viz_process = multiprocessing.Process( + process_context = multiprocessing.get_context("spawn") + + viz_process = process_context.Process( target=run_process, daemon=False, args=run_process_args, diff --git a/package/kedro_viz/utils.py b/package/kedro_viz/utils.py index fb9fd0e9f3..a0a4a5abce 100644 --- a/package/kedro_viz/utils.py +++ b/package/kedro_viz/utils.py @@ -57,8 +57,3 @@ def _strip_transcoding(element: str) -> str: def is_dataset_param(dataset_name: str) -> bool: """Return whether a dataset is a parameter""" return dataset_name.lower().startswith("params:") or dataset_name == "parameters" - - -def file_extension_filter(_, file_path: str) -> bool: - """Determine if a given file path ends with one of the specified extensions.""" - return file_path.endswith((".yml", ".yaml", ".py", ".json")) diff --git a/package/tests/test_autoreload_file_filter.py b/package/tests/test_autoreload_file_filter.py new file mode 100644 index 0000000000..ab3ae01ce9 --- /dev/null +++ b/package/tests/test_autoreload_file_filter.py @@ -0,0 +1,116 @@ +import logging +import shutil +import tempfile +from pathlib import Path +from unittest.mock import patch + +import pytest +from watchfiles import Change + +from kedro_viz.autoreload_file_filter import AutoreloadFileFilter + +logger = logging.getLogger(__name__) + + +@pytest.fixture +def test_environment(): + # Create a temporary directory + test_dir = tempfile.mkdtemp() + yield test_dir + # Remove temp directory + shutil.rmtree(test_dir) + + +@pytest.fixture +def file_filter(test_environment): + test_dir = Path(test_environment) + # Create a .gitignore file + gitignore_path = test_dir / ".gitignore" + gitignore_path.write_text("ignored.py\n") + + # Initialize the filter with the test directory as base_path + return AutoreloadFileFilter(base_path=test_dir) + + +def test_no_gitignore(test_environment): + test_dir = Path(test_environment) + gitignored_file = test_dir / "ignored.py" + gitignored_file.touch() + + # Initialize the filter without a .gitignore file + gitignore_path = test_dir / ".gitignore" + if gitignore_path.exists(): + gitignore_path.unlink() + file_filter = AutoreloadFileFilter(base_path=test_dir) + + result = file_filter(Change.modified, str(gitignored_file)) + assert result, "File should pass the filter when .gitignore is missing" + + +def test_gitignore_exception(file_filter, test_environment): + test_dir = Path(test_environment) + allowed_file = test_dir / "test.py" + allowed_file.touch() + + with patch( + "pathspec.PathSpec.match_file", side_effect=Exception("Mocked exception") + ): + result = file_filter(Change.modified, str(allowed_file)) + assert result, "Filter should pass the file if .gitignore matching fails" + + +def test_allowed_file(file_filter, test_environment): + test_dir = Path(test_environment) + allowed_file = test_dir / "test.py" + allowed_file.touch() + + result = file_filter(Change.modified, str(allowed_file)) + assert result, "Allowed file should pass the filter" + + +def test_disallowed_file(file_filter, test_environment): + test_dir = Path(test_environment) + disallowed_file = test_dir / "test.txt" + disallowed_file.touch() + + result = file_filter(Change.modified, str(disallowed_file)) + assert not result, "Disallowed file should not pass the filter" + + +def test_gitignored_file(file_filter, test_environment): + test_dir = Path(test_environment) + gitignored_file = test_dir / "ignored.py" + gitignored_file.touch() + + result = file_filter(Change.modified, str(gitignored_file)) + assert not result, "Gitignored file should not pass the filter" + + +def test_non_relative_path(file_filter, test_environment): + original_cwd = Path.cwd().parent # Go up one directory + outside_file = original_cwd / "outside.py" + outside_file.touch() + + result = file_filter(Change.modified, str(outside_file)) + assert not result, "File outside the CWD should not pass the filter" + + # Cleanup + outside_file.unlink() + + +def test_no_allowed_extension(file_filter, test_environment): + test_dir = Path(test_environment) + no_extension_file = test_dir / "no_extension" + no_extension_file.touch() + + result = file_filter(Change.modified, str(no_extension_file)) + assert not result, "File without allowed extension should not pass the filter" + + +def test_directory_path(file_filter, test_environment): + test_dir = Path(test_environment) + directory_path = test_dir / "some_directory" + directory_path.mkdir() + + result = file_filter(Change.modified, str(directory_path)) + assert not result, "Directories should not pass the filter" From cc2ea9ef692b3641799e00022134978f04d8ccf7 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Tue, 22 Oct 2024 15:18:16 +0100 Subject: [PATCH 11/22] Test fix Signed-off-by: Jitendra Gundaniya --- .../tests/test_launchers/test_cli/test_run.py | 4 ++-- package/tests/test_utils.py | 19 ------------------- 2 files changed, 2 insertions(+), 21 deletions(-) delete mode 100644 package/tests/test_utils.py diff --git a/package/tests/test_launchers/test_cli/test_run.py b/package/tests/test_launchers/test_cli/test_run.py index c358647bb3..cfc60c52d9 100644 --- a/package/tests/test_launchers/test_cli/test_run.py +++ b/package/tests/test_launchers/test_cli/test_run.py @@ -7,11 +7,11 @@ from watchfiles import run_process from kedro_viz import __version__ +from kedro_viz.autoreload_file_filter import AutoreloadFileFilter from kedro_viz.launchers.cli import main from kedro_viz.launchers.cli.run import _VIZ_PROCESSES from kedro_viz.launchers.utils import _PYPROJECT from kedro_viz.server import run_server -from kedro_viz.utils import file_extension_filter @pytest.fixture @@ -375,7 +375,7 @@ def test_kedro_viz_command_with_autoreload( "extra_params": {}, "is_lite": False, }, - "watch_filter": file_extension_filter, + "watch_filter": AutoreloadFileFilter(), } process_init.assert_called_once_with( diff --git a/package/tests/test_utils.py b/package/tests/test_utils.py deleted file mode 100644 index 414976ed05..0000000000 --- a/package/tests/test_utils.py +++ /dev/null @@ -1,19 +0,0 @@ -import pytest - -from kedro_viz.utils import file_extension_filter - - -@pytest.mark.parametrize( - "file_path, expected", - [ - ("config.yml", True), - ("config.yaml", True), - ("script.py", True), - ("data.json", True), - ("image.png", False), - ("document.txt", False), - ("archive.zip", False), - ], -) -def test_file_extension_filter(file_path, expected): - assert file_extension_filter(None, file_path) == expected From ca9239ace61eb399a361add5bb7faf34b1db7912 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Tue, 22 Oct 2024 17:17:15 +0100 Subject: [PATCH 12/22] Test fix Signed-off-by: Jitendra Gundaniya --- .../tests/test_launchers/test_cli/test_run.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/package/tests/test_launchers/test_cli/test_run.py b/package/tests/test_launchers/test_cli/test_run.py index cfc60c52d9..4ad2e11235 100644 --- a/package/tests/test_launchers/test_cli/test_run.py +++ b/package/tests/test_launchers/test_cli/test_run.py @@ -206,7 +206,11 @@ def test_kedro_viz_command_run_server( patched_check_viz_up, patched_start_browser, ): - process_init = mocker.patch("multiprocessing.Process") + # process_init = mocker.patch("multiprocessing.Process") + mock_process_context = mocker.patch("multiprocessing.get_context") + mock_context_instance = mocker.Mock() + mock_process_context.return_value = mock_context_instance + mock_process = mocker.patch.object(mock_context_instance, "Process") runner = CliRunner() # Reduce the timeout argument from 600 to 1 to make test run faster. @@ -223,7 +227,7 @@ def test_kedro_viz_command_run_server( with runner.isolated_filesystem(): runner.invoke(main.viz_cli, command_options) - process_init.assert_called_once_with( + mock_process.assert_called_once_with( target=run_server, daemon=False, kwargs={**run_server_args} ) @@ -343,7 +347,11 @@ def test_kedro_viz_command_should_not_log_if_pypi_is_down( def test_kedro_viz_command_with_autoreload( self, mocker, mock_project_path, patched_check_viz_up, patched_start_browser ): - process_init = mocker.patch("multiprocessing.Process") + # process_init = mocker.patch("multiprocessing.Process") + mock_process_context = mocker.patch("multiprocessing.get_context") + mock_context_instance = mocker.Mock() + mock_process_context.return_value = mock_context_instance + mock_process = mocker.patch.object(mock_context_instance, "Process") # Reduce the timeout argument from 600 to 1 to make test run faster. mocker.patch( @@ -378,7 +386,7 @@ def test_kedro_viz_command_with_autoreload( "watch_filter": AutoreloadFileFilter(), } - process_init.assert_called_once_with( + mock_process.assert_called_once_with( target=run_process, daemon=False, args=run_process_args, From 3119a6101ac733965c81a0ba59eac1b80b2ce9bb Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 23 Oct 2024 12:08:19 +0100 Subject: [PATCH 13/22] lint and unit test fix Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/autoreload_file_filter.py | 34 +++++++++++-------- .../tests/test_launchers/test_cli/test_run.py | 15 ++++---- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py index 8d407f49f0..e8111ea003 100644 --- a/package/kedro_viz/autoreload_file_filter.py +++ b/package/kedro_viz/autoreload_file_filter.py @@ -1,3 +1,8 @@ +""" +This module provides a custom file filter for autoreloading that filters out files based on allowed +file extensions and patterns specified in a .gitignore file. +""" + import logging from pathlib import Path from typing import Optional, Set @@ -19,8 +24,10 @@ class AutoreloadFileFilter(DefaultFilter): def __init__(self, base_path: Optional[Path] = None): """ Initialize the AutoreloadFileFilter. + Args: - base_path (Optional[Path]): The base path to set as the current working directory for the filter. + base_path (Optional[Path]): The base path to set as the current working directory + for the filter. """ self.cwd = base_path or Path.cwd() @@ -30,7 +37,7 @@ def __init__(self, base_path: Optional[Path] = None): # Load .gitignore patterns gitignore_path = self.cwd / ".gitignore" try: - with open(gitignore_path, "r") as gitignore_file: + with open(gitignore_path, "r", encoding="utf-8") as gitignore_file: ignore_patterns = gitignore_file.read().splitlines() self.gitignore_spec = pathspec.PathSpec.from_lines( "gitwildmatch", ignore_patterns @@ -41,14 +48,16 @@ def __init__(self, base_path: Optional[Path] = None): def __call__(self, change: Change, path: str) -> bool: """ Determine whether a file change should be processed. + Args: change (Change): The type of change detected. path (str): The path to the file that changed. + Returns: bool: True if the file should be processed, False otherwise. """ if not super().__call__(change, path): - logger.debug(f"Filtered out by DefaultFilter: {path}") + logger.debug("Filtered out by DefaultFilter: %s", path) return False path_obj = Path(path) @@ -57,23 +66,20 @@ def __call__(self, change: Change, path: str) -> bool: try: relative_path = path_obj.resolve().relative_to(self.cwd.resolve()) except ValueError: - logger.debug(f"Path not relative to CWD: {path}") + logger.debug("Path not relative to CWD: %s", path) return False try: - if self.gitignore_spec and self.gitignore_spec.match_file( - str(relative_path) - ): - logger.debug(f"Filtered out by .gitignore: {relative_path}") + if self.gitignore_spec and self.gitignore_spec.match_file(str(relative_path)): + logger.debug("Filtered out by .gitignore: %s", relative_path) return False - except Exception as e: - logger.debug(f"Exception during .gitignore matching: {e}") + except Exception as exc: + logger.debug("Exception during .gitignore matching: %s", exc) return True # Pass the file if .gitignore matching fails # Include only files with allowed extensions if path_obj.suffix in self.allowed_extensions: - logger.debug(f"Allowed file: {path}") + logger.debug("Allowed file: %s", path) return True - else: - logger.debug(f"Filtered out by allowed_extensions: {path_obj.suffix}") - return False + logger.debug("Filtered out by allowed_extensions: %s", path_obj.suffix) + return False diff --git a/package/tests/test_launchers/test_cli/test_run.py b/package/tests/test_launchers/test_cli/test_run.py index 4ad2e11235..86adae92f6 100644 --- a/package/tests/test_launchers/test_cli/test_run.py +++ b/package/tests/test_launchers/test_cli/test_run.py @@ -206,7 +206,6 @@ def test_kedro_viz_command_run_server( patched_check_viz_up, patched_start_browser, ): - # process_init = mocker.patch("multiprocessing.Process") mock_process_context = mocker.patch("multiprocessing.get_context") mock_context_instance = mocker.Mock() mock_process_context.return_value = mock_context_instance @@ -345,13 +344,15 @@ def test_kedro_viz_command_should_not_log_if_pypi_is_down( mock_click_echo.assert_has_calls(mock_click_echo_calls) def test_kedro_viz_command_with_autoreload( - self, mocker, mock_project_path, patched_check_viz_up, patched_start_browser + self, mocker, tmp_path, patched_check_viz_up, patched_start_browser ): - # process_init = mocker.patch("multiprocessing.Process") mock_process_context = mocker.patch("multiprocessing.get_context") mock_context_instance = mocker.Mock() mock_process_context.return_value = mock_context_instance mock_process = mocker.patch.object(mock_context_instance, "Process") + mock_tmp_path = tmp_path / "tmp" + mock_tmp_path.mkdir() + mock_path = mock_tmp_path / "project_path" # Reduce the timeout argument from 600 to 1 to make test run faster. mocker.patch( @@ -360,13 +361,13 @@ def test_kedro_viz_command_with_autoreload( # Mock finding kedro project mocker.patch( "kedro_viz.launchers.utils._find_kedro_project", - return_value=mock_project_path, + return_value=mock_path, ) runner = CliRunner() with runner.isolated_filesystem(): runner.invoke(main.viz_cli, ["viz", "run", "--autoreload"]) - run_process_args = [str(mock_project_path)] + run_process_args = [str(mock_path)] run_process_kwargs = { "target": run_server, "kwargs": { @@ -376,14 +377,14 @@ def test_kedro_viz_command_with_autoreload( "save_file": None, "pipeline_name": None, "env": None, + "project_path": mock_path, "autoreload": True, - "project_path": mock_project_path, "include_hooks": False, "package_name": None, "extra_params": {}, "is_lite": False, }, - "watch_filter": AutoreloadFileFilter(), + "watch_filter": mocker.ANY, } mock_process.assert_called_once_with( From cb37822c566aeedea31850cf2d28ee0f94eb0513 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 23 Oct 2024 12:48:37 +0100 Subject: [PATCH 14/22] Test fix Signed-off-by: Jitendra Gundaniya --- package/features/steps/lower_requirements.txt | 1 + package/requirements.txt | 1 + package/test_requirements.txt | 1 + package/tests/test_autoreload_file_filter.py | 13 ++++++++++++- 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/package/features/steps/lower_requirements.txt b/package/features/steps/lower_requirements.txt index 6e12855128..91f69f375e 100644 --- a/package/features/steps/lower_requirements.txt +++ b/package/features/steps/lower_requirements.txt @@ -16,3 +16,4 @@ secure==0.3.0 # numpy 2.0 breaks with old versions of pandas and this # could be removed when the lowest version supported is updated numpy==1.26.4 +pathspec=0.12.1 diff --git a/package/requirements.txt b/package/requirements.txt index f16575bf6a..16c7890f5d 100644 --- a/package/requirements.txt +++ b/package/requirements.txt @@ -16,3 +16,4 @@ sqlalchemy>=1.4, <3 strawberry-graphql>=0.192.0, <1.0 uvicorn[standard]>=0.30.0, <1.0 watchfiles>=0.24.0 +pathspec>=0.12.1 \ No newline at end of file diff --git a/package/test_requirements.txt b/package/test_requirements.txt index 14741ab0ea..9d872959db 100644 --- a/package/test_requirements.txt +++ b/package/test_requirements.txt @@ -23,6 +23,7 @@ sqlalchemy-stubs~=0.4 strawberry-graphql[cli]>=0.99.0, <1.0 trufflehog~=2.2 httpx~=0.27.0 +pathspec>=0.12.1 # mypy types-aiofiles==0.1.3 diff --git a/package/tests/test_autoreload_file_filter.py b/package/tests/test_autoreload_file_filter.py index ab3ae01ce9..47a82de927 100644 --- a/package/tests/test_autoreload_file_filter.py +++ b/package/tests/test_autoreload_file_filter.py @@ -5,7 +5,7 @@ from unittest.mock import patch import pytest -from watchfiles import Change +from watchfiles import Change, DefaultFilter from kedro_viz.autoreload_file_filter import AutoreloadFileFilter @@ -114,3 +114,14 @@ def test_directory_path(file_filter, test_environment): result = file_filter(Change.modified, str(directory_path)) assert not result, "Directories should not pass the filter" + +def test_filtered_out_by_default_filter(file_filter, test_environment, mocker): + test_dir = Path(test_environment) + filtered_file = test_dir / "filtered.py" + filtered_file.touch() + + # Mock the super().__call__ method to return False + mocker.patch.object(DefaultFilter, '__call__', return_value=False) + + result = file_filter(Change.modified, str(filtered_file)) + assert not result, "File should be filtered out by DefaultFilter" From 8a4ea99dc645e286c6affd5c598d9f5f5f18a0c5 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 23 Oct 2024 13:00:16 +0100 Subject: [PATCH 15/22] Lint fix Signed-off-by: Jitendra Gundaniya --- package/features/steps/lower_requirements.txt | 2 +- package/kedro_viz/autoreload_file_filter.py | 4 +++- package/tests/test_autoreload_file_filter.py | 5 +++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/package/features/steps/lower_requirements.txt b/package/features/steps/lower_requirements.txt index 91f69f375e..ab38585acf 100644 --- a/package/features/steps/lower_requirements.txt +++ b/package/features/steps/lower_requirements.txt @@ -16,4 +16,4 @@ secure==0.3.0 # numpy 2.0 breaks with old versions of pandas and this # could be removed when the lowest version supported is updated numpy==1.26.4 -pathspec=0.12.1 +pathspec==0.12.1 diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py index e8111ea003..88f1429a7d 100644 --- a/package/kedro_viz/autoreload_file_filter.py +++ b/package/kedro_viz/autoreload_file_filter.py @@ -70,7 +70,9 @@ def __call__(self, change: Change, path: str) -> bool: return False try: - if self.gitignore_spec and self.gitignore_spec.match_file(str(relative_path)): + if self.gitignore_spec and self.gitignore_spec.match_file( + str(relative_path) + ): logger.debug("Filtered out by .gitignore: %s", relative_path) return False except Exception as exc: diff --git a/package/tests/test_autoreload_file_filter.py b/package/tests/test_autoreload_file_filter.py index 47a82de927..52682c57c8 100644 --- a/package/tests/test_autoreload_file_filter.py +++ b/package/tests/test_autoreload_file_filter.py @@ -115,13 +115,14 @@ def test_directory_path(file_filter, test_environment): result = file_filter(Change.modified, str(directory_path)) assert not result, "Directories should not pass the filter" + def test_filtered_out_by_default_filter(file_filter, test_environment, mocker): test_dir = Path(test_environment) filtered_file = test_dir / "filtered.py" filtered_file.touch() # Mock the super().__call__ method to return False - mocker.patch.object(DefaultFilter, '__call__', return_value=False) + mocker.patch.object(DefaultFilter, "__call__", return_value=False) result = file_filter(Change.modified, str(filtered_file)) - assert not result, "File should be filtered out by DefaultFilter" + assert not result, "File should be filtered out by DefaultFilter" From 19c6ef40278e7a78230f6c8dc2a6800729e0de6b Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 23 Oct 2024 13:26:04 +0100 Subject: [PATCH 16/22] Lint fix Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/autoreload_file_filter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py index 88f1429a7d..fa31cdf6d8 100644 --- a/package/kedro_viz/autoreload_file_filter.py +++ b/package/kedro_viz/autoreload_file_filter.py @@ -75,6 +75,7 @@ def __call__(self, change: Change, path: str) -> bool: ): logger.debug("Filtered out by .gitignore: %s", relative_path) return False + # pylint: disable=broad-exception-caught except Exception as exc: logger.debug("Exception during .gitignore matching: %s", exc) return True # Pass the file if .gitignore matching fails From eb33dfa87587237a4e0fa89f19df33eb095200f3 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Wed, 23 Oct 2024 13:53:26 +0100 Subject: [PATCH 17/22] Fix import issue Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/autoreload_file_filter.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py index fa31cdf6d8..0dc31d0f0c 100644 --- a/package/kedro_viz/autoreload_file_filter.py +++ b/package/kedro_viz/autoreload_file_filter.py @@ -8,6 +8,7 @@ from typing import Optional, Set import pathspec +from pathspec import PathSpec from watchfiles import Change, DefaultFilter logger = logging.getLogger(__name__) @@ -39,7 +40,7 @@ def __init__(self, base_path: Optional[Path] = None): try: with open(gitignore_path, "r", encoding="utf-8") as gitignore_file: ignore_patterns = gitignore_file.read().splitlines() - self.gitignore_spec = pathspec.PathSpec.from_lines( + self.gitignore_spec: Optional[PathSpec] = pathspec.PathSpec.from_lines( "gitwildmatch", ignore_patterns ) except FileNotFoundError: From 2785250ada66b97c5fb9a195cbf224fe6a43ca49 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Fri, 25 Oct 2024 20:34:53 +0100 Subject: [PATCH 18/22] Extra import removed Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/autoreload_file_filter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py index 0dc31d0f0c..88ed49ac89 100644 --- a/package/kedro_viz/autoreload_file_filter.py +++ b/package/kedro_viz/autoreload_file_filter.py @@ -7,7 +7,6 @@ from pathlib import Path from typing import Optional, Set -import pathspec from pathspec import PathSpec from watchfiles import Change, DefaultFilter @@ -40,7 +39,7 @@ def __init__(self, base_path: Optional[Path] = None): try: with open(gitignore_path, "r", encoding="utf-8") as gitignore_file: ignore_patterns = gitignore_file.read().splitlines() - self.gitignore_spec: Optional[PathSpec] = pathspec.PathSpec.from_lines( + self.gitignore_spec: Optional[PathSpec] = PathSpec.from_lines( "gitwildmatch", ignore_patterns ) except FileNotFoundError: From 2c941cc25f84d19c2bb2f163c20e2aecc213096e Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Tue, 29 Oct 2024 13:24:20 +0000 Subject: [PATCH 19/22] tmp_path fix for tests Signed-off-by: Jitendra Gundaniya --- package/tests/test_autoreload_file_filter.py | 62 +++++++------------- 1 file changed, 22 insertions(+), 40 deletions(-) diff --git a/package/tests/test_autoreload_file_filter.py b/package/tests/test_autoreload_file_filter.py index 52682c57c8..60b702eba6 100644 --- a/package/tests/test_autoreload_file_filter.py +++ b/package/tests/test_autoreload_file_filter.py @@ -13,43 +13,31 @@ @pytest.fixture -def test_environment(): - # Create a temporary directory - test_dir = tempfile.mkdtemp() - yield test_dir - # Remove temp directory - shutil.rmtree(test_dir) - - -@pytest.fixture -def file_filter(test_environment): - test_dir = Path(test_environment) +def file_filter(tmp_path): # Create a .gitignore file - gitignore_path = test_dir / ".gitignore" + gitignore_path = tmp_path / ".gitignore" gitignore_path.write_text("ignored.py\n") # Initialize the filter with the test directory as base_path - return AutoreloadFileFilter(base_path=test_dir) + return AutoreloadFileFilter(base_path=tmp_path) -def test_no_gitignore(test_environment): - test_dir = Path(test_environment) - gitignored_file = test_dir / "ignored.py" +def test_no_gitignore(tmp_path): + gitignored_file = tmp_path / "ignored.py" gitignored_file.touch() # Initialize the filter without a .gitignore file - gitignore_path = test_dir / ".gitignore" + gitignore_path = tmp_path / ".gitignore" if gitignore_path.exists(): gitignore_path.unlink() - file_filter = AutoreloadFileFilter(base_path=test_dir) + file_filter = AutoreloadFileFilter(base_path=tmp_path) result = file_filter(Change.modified, str(gitignored_file)) assert result, "File should pass the filter when .gitignore is missing" -def test_gitignore_exception(file_filter, test_environment): - test_dir = Path(test_environment) - allowed_file = test_dir / "test.py" +def test_gitignore_exception(file_filter, tmp_path): + allowed_file = tmp_path / "test.py" allowed_file.touch() with patch( @@ -59,34 +47,31 @@ def test_gitignore_exception(file_filter, test_environment): assert result, "Filter should pass the file if .gitignore matching fails" -def test_allowed_file(file_filter, test_environment): - test_dir = Path(test_environment) - allowed_file = test_dir / "test.py" +def test_allowed_file(file_filter, tmp_path): + allowed_file = tmp_path / "test.py" allowed_file.touch() result = file_filter(Change.modified, str(allowed_file)) assert result, "Allowed file should pass the filter" -def test_disallowed_file(file_filter, test_environment): - test_dir = Path(test_environment) - disallowed_file = test_dir / "test.txt" +def test_disallowed_file(file_filter, tmp_path): + disallowed_file = tmp_path / "test.txt" disallowed_file.touch() result = file_filter(Change.modified, str(disallowed_file)) assert not result, "Disallowed file should not pass the filter" -def test_gitignored_file(file_filter, test_environment): - test_dir = Path(test_environment) - gitignored_file = test_dir / "ignored.py" +def test_gitignored_file(file_filter, tmp_path): + gitignored_file = tmp_path / "ignored.py" gitignored_file.touch() result = file_filter(Change.modified, str(gitignored_file)) assert not result, "Gitignored file should not pass the filter" -def test_non_relative_path(file_filter, test_environment): +def test_non_relative_path(file_filter): original_cwd = Path.cwd().parent # Go up one directory outside_file = original_cwd / "outside.py" outside_file.touch() @@ -98,27 +83,24 @@ def test_non_relative_path(file_filter, test_environment): outside_file.unlink() -def test_no_allowed_extension(file_filter, test_environment): - test_dir = Path(test_environment) - no_extension_file = test_dir / "no_extension" +def test_no_allowed_extension(file_filter, tmp_path): + no_extension_file = tmp_path / "no_extension" no_extension_file.touch() result = file_filter(Change.modified, str(no_extension_file)) assert not result, "File without allowed extension should not pass the filter" -def test_directory_path(file_filter, test_environment): - test_dir = Path(test_environment) - directory_path = test_dir / "some_directory" +def test_directory_path(file_filter, tmp_path): + directory_path = tmp_path / "some_directory" directory_path.mkdir() result = file_filter(Change.modified, str(directory_path)) assert not result, "Directories should not pass the filter" -def test_filtered_out_by_default_filter(file_filter, test_environment, mocker): - test_dir = Path(test_environment) - filtered_file = test_dir / "filtered.py" +def test_filtered_out_by_default_filter(file_filter, tmp_path, mocker): + filtered_file = tmp_path / "filtered.py" filtered_file.touch() # Mock the super().__call__ method to return False From 277098d6c3daf82720abd8d0ef4d6e88188ecfa9 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Tue, 29 Oct 2024 13:48:27 +0000 Subject: [PATCH 20/22] Lint fix Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/autoreload_file_filter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py index 88ed49ac89..26cd493fda 100644 --- a/package/kedro_viz/autoreload_file_filter.py +++ b/package/kedro_viz/autoreload_file_filter.py @@ -75,7 +75,7 @@ def __call__(self, change: Change, path: str) -> bool: ): logger.debug("Filtered out by .gitignore: %s", relative_path) return False - # pylint: disable=broad-exception-caught + # ruff: noqa: BLE001 except Exception as exc: logger.debug("Exception during .gitignore matching: %s", exc) return True # Pass the file if .gitignore matching fails From b52ec5d52c6f5e1b43d11900aa1df51c6eb5d2b5 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Tue, 29 Oct 2024 14:38:08 +0000 Subject: [PATCH 21/22] Tests with docstring Signed-off-by: Jitendra Gundaniya --- package/tests/test_autoreload_file_filter.py | 31 ++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/package/tests/test_autoreload_file_filter.py b/package/tests/test_autoreload_file_filter.py index 60b702eba6..d5c9fb2ff7 100644 --- a/package/tests/test_autoreload_file_filter.py +++ b/package/tests/test_autoreload_file_filter.py @@ -14,6 +14,10 @@ @pytest.fixture def file_filter(tmp_path): + """ + Fixture to create a temporary .gitignore file and initialize the AutoreloadFileFilter + with the test directory as the base path. + """ # Create a .gitignore file gitignore_path = tmp_path / ".gitignore" gitignore_path.write_text("ignored.py\n") @@ -23,6 +27,9 @@ def file_filter(tmp_path): def test_no_gitignore(tmp_path): + """ + Test that a file passes the filter when the .gitignore file is missing. + """ gitignored_file = tmp_path / "ignored.py" gitignored_file.touch() @@ -37,6 +44,9 @@ def test_no_gitignore(tmp_path): def test_gitignore_exception(file_filter, tmp_path): + """ + Test that a file passes the filter if an exception occurs during .gitignore matching. + """ allowed_file = tmp_path / "test.py" allowed_file.touch() @@ -48,6 +58,9 @@ def test_gitignore_exception(file_filter, tmp_path): def test_allowed_file(file_filter, tmp_path): + """ + Test that a file with an allowed extension passes the filter. + """ allowed_file = tmp_path / "test.py" allowed_file.touch() @@ -56,6 +69,9 @@ def test_allowed_file(file_filter, tmp_path): def test_disallowed_file(file_filter, tmp_path): + """ + Test that a file with a disallowed extension does not pass the filter. + """ disallowed_file = tmp_path / "test.txt" disallowed_file.touch() @@ -64,6 +80,9 @@ def test_disallowed_file(file_filter, tmp_path): def test_gitignored_file(file_filter, tmp_path): + """ + Test that a file listed in the .gitignore file does not pass the filter. + """ gitignored_file = tmp_path / "ignored.py" gitignored_file.touch() @@ -72,6 +91,9 @@ def test_gitignored_file(file_filter, tmp_path): def test_non_relative_path(file_filter): + """ + Test that a file outside the current working directory does not pass the filter. + """ original_cwd = Path.cwd().parent # Go up one directory outside_file = original_cwd / "outside.py" outside_file.touch() @@ -84,6 +106,9 @@ def test_non_relative_path(file_filter): def test_no_allowed_extension(file_filter, tmp_path): + """ + Test that a file without an allowed extension does not pass the filter. + """ no_extension_file = tmp_path / "no_extension" no_extension_file.touch() @@ -92,6 +117,9 @@ def test_no_allowed_extension(file_filter, tmp_path): def test_directory_path(file_filter, tmp_path): + """ + Test that a directory does not pass the filter. + """ directory_path = tmp_path / "some_directory" directory_path.mkdir() @@ -100,6 +128,9 @@ def test_directory_path(file_filter, tmp_path): def test_filtered_out_by_default_filter(file_filter, tmp_path, mocker): + """ + Test that a file is filtered out by the DefaultFilter. + """ filtered_file = tmp_path / "filtered.py" filtered_file.touch() From 14f38df155510d76dfa8dcb6d88dad6e5685edb2 Mon Sep 17 00:00:00 2001 From: Jitendra Gundaniya Date: Thu, 31 Oct 2024 14:14:48 +0000 Subject: [PATCH 22/22] Moved to GitIgnoreSpec class Signed-off-by: Jitendra Gundaniya --- package/kedro_viz/autoreload_file_filter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package/kedro_viz/autoreload_file_filter.py b/package/kedro_viz/autoreload_file_filter.py index 26cd493fda..f8b13c6237 100644 --- a/package/kedro_viz/autoreload_file_filter.py +++ b/package/kedro_viz/autoreload_file_filter.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import Optional, Set -from pathspec import PathSpec +from pathspec import GitIgnoreSpec from watchfiles import Change, DefaultFilter logger = logging.getLogger(__name__) @@ -39,7 +39,7 @@ def __init__(self, base_path: Optional[Path] = None): try: with open(gitignore_path, "r", encoding="utf-8") as gitignore_file: ignore_patterns = gitignore_file.read().splitlines() - self.gitignore_spec: Optional[PathSpec] = PathSpec.from_lines( + self.gitignore_spec: Optional[GitIgnoreSpec] = GitIgnoreSpec.from_lines( "gitwildmatch", ignore_patterns ) except FileNotFoundError: