Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve all path in Kedro #3742

Merged
merged 11 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@

tmp/
# CMake
cmake-build-debug/

Expand Down
11 changes: 7 additions & 4 deletions kedro/framework/startup.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
"""This module provides metadata for a Kedro project."""
from __future__ import annotations

import os
import sys
from pathlib import Path
from typing import NamedTuple, Union
from typing import NamedTuple

import toml

Expand Down Expand Up @@ -34,7 +36,7 @@ def _version_mismatch_error(kedro_init_version: str) -> str:
)


def _get_project_metadata(project_path: Union[str, Path]) -> ProjectMetadata:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hesitate a bit on whether it should accept both str | Path. As you can see it breaks one of our test.

On the other hand it's much cleaner to keep this path conversion happen at the beginning, i.e. bootstrap_project. Otherwise we are doing the same conversion over and over, and this is the only public function.

def _get_project_metadata(project_path: Path) -> ProjectMetadata:
"""Read project metadata from `<project_root>/pyproject.toml` config file,
under the `[tool.kedro]` section.

Expand All @@ -50,7 +52,6 @@ def _get_project_metadata(project_path: Union[str, Path]) -> ProjectMetadata:
Returns:
A named tuple that contains project metadata.
"""
project_path = Path(project_path).expanduser().resolve()
pyproject_toml = project_path / _PYPROJECT

if not pyproject_toml.is_file():
Expand Down Expand Up @@ -144,10 +145,12 @@ def _add_src_to_path(source_dir: Path, project_path: Path) -> None:
os.environ["PYTHONPATH"] = f"{str(source_dir)}{sep}{python_path}"


def bootstrap_project(project_path: Path) -> ProjectMetadata:
def bootstrap_project(project_path: str | Path) -> ProjectMetadata:
"""Run setup required at the beginning of the workflow
when running in project mode, and return project metadata.
"""

project_path = Path(project_path).expanduser().resolve()
metadata = _get_project_metadata(project_path)
_add_src_to_path(metadata.source_dir, project_path)
configure_project(metadata.package_name)
Expand Down
15 changes: 14 additions & 1 deletion tests/framework/test_startup.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def test_toml_invalid_format(self, tmp_path):
toml_path.write_text("!!") # Invalid TOML
pattern = "Failed to parse 'pyproject.toml' file"
with pytest.raises(RuntimeError, match=re.escape(pattern)):
_get_project_metadata(str(tmp_path))
_get_project_metadata(tmp_path)

def test_valid_toml_file(self, mocker):
mocker.patch.object(Path, "is_file", return_value=True)
Expand Down Expand Up @@ -275,6 +275,19 @@ def test_non_existent_source_path(self, tmp_path):
with pytest.raises(NotADirectoryError, match=pattern):
_validate_source_path(source_path, tmp_path.resolve())

@pytest.mark.parametrize(
"source_dir", [".", "src", "./src", "src/nested", "src/nested/nested"]
)
def test_symlink_source_path(self, tmp_path, source_dir):
source_path = (tmp_path / source_dir).resolve()
source_path.mkdir(parents=True, exist_ok=True)

fake_path = tmp_path / "../" / ".path_does_not_exist"
fake_path.symlink_to(source_path)

_validate_source_path(fake_path, tmp_path.resolve())
os.remove(fake_path)


class TestBootstrapProject:
def test_bootstrap_project(self, monkeypatch, tmp_path):
Expand Down