Make kedro commands work from inside subdirectories in project #3683

ankatiyar · 2024-03-05T13:35:24Z

Description

Development notes

Move _find_kedro_project() and _is_project() to kedro.utils so it can be used both by KedroCLI and kedro.ipython extension
Update _find_kedro_project() to use Path.parents
Update project_path for KedroCLI and KedroSession to use _find_kedro_project()
Added e2e test for this change

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

Read the contributing guidelines
Signed off each commit with a Developer Certificate of Origin (DCO)
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added a description of this change in the RELEASE.md file
Added tests to cover my changes
Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

datajoely · 2024-03-06T10:27:43Z

Super good push!

Possibly a different ticket - but it would be nice to catch some of the common within project commands (which 90% of the time will be run) and say something useful like "You are not within a Kedro project directory, please navigate there to use {command}" rather than the pretty confusing error users get today...

❯ kedro run
Usage: kedro [OPTIONS] COMMAND [ARGS]...
Try 'kedro -h' for help.

Error: No such command 'run'.

noklam · 2024-03-06T10:33:54Z

@datajoely #3680

noklam

I did a manual test with arbitary deep directory with kedro run,kedro ipython, all works well. This is such a small change but big improvement in the developer experience.

It checks all the parents, which I think it's good. Alternatively we are try to traverse the directory up, we run into risks that we execute other kedro project accidentally.

Out of scope:

@rashidakanchwala I test viz command and it's failing.

noklam · 2024-03-06T15:53:23Z

Since this breaks kedro-viz, I also send a question in Slack. In some sense we may consider this as a breaking change, mainly to plugin developer. Though I don't think this should stop us from releasing this in 0.19, but just in case we should check with some plugin developers particularly if they rely on the fact that command are running from project root.

ravi-kumar-pilla · 2024-03-06T16:01:46Z

Since this breaks kedro-viz, I also send a question in Slack. In some sense we may consider this as a breaking change, mainly to plugin developer. Though I don't think this should stop us from releasing this in 0.19, but just in case we should check with some plugin developers particularly if they rely on the fact that command are running from project root.

Hi Nok, Currently the commands kedro viz run and kedro viz build default to current working directory as Kedro project path. We are introducing an option for --project-path in kedro viz run here. We can also plan on extending this to other cli commands. Thank you

merelcht

The implementation looks all good and I've tested it successfully as well 👍 I agree with @noklam that even though it can be considered breaking, it should be shipped in the 0.19.x series.

On the viz issue we could either change the logic there, or at the least make the error message clearer when running Viz in a subdirectory.

merelcht · 2024-03-06T16:49:53Z

kedro/framework/session/session.py

@@ -104,7 +105,9 @@ def __init__(  # noqa: PLR0913
        save_on_close: bool = False,
        conf_source: str | None = None,
    ):
-        self._project_path = Path(project_path or Path.cwd()).resolve()
+        self._project_path = Path(
+            project_path or _find_kedro_project(Path.cwd()).resolve() or Path.cwd()


Previously we called .resolve() on all options project_path as well as Path.cwd() and now only on _find_kedro_project(Path.cwd()).resolve(), why did that change?

merelcht · 2024-03-06T16:51:47Z

RELEASE.md

@@ -1,6 +1,7 @@
 # Upcoming Release 0.19.4

 ## Major features and improvements
+* Kedro commands now work from any subdirectory within a Kedro project.


_is_project and _find_kedro_project are private methods, but it might be worth mentioning in the release notes section "Breaking changes to the API" that these were moved to kedro.utils, just in case people are using them.

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

Signed-off-by: Ankita Katiyar <110245118+ankatiyar@users.noreply.github.com>

noklam

Nice change! It makes working with Kedro in terminal so much smoother.

merelcht

LGTM! 😄

astrojuanlu · 2024-03-07T21:04:38Z

Wow, this was so fast I didn't even have time to review it. Well done everyone ❤️

noklam · 2024-03-08T00:03:27Z

ankatiyar and others added 2 commits March 5, 2024 13:34

Move _find_kedro_project to utils

c73ef68

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

Merge branch 'main' into kedro-commands-in-subdirectories

d9d7c44

ankatiyar linked an issue Mar 5, 2024 that may be closed by this pull request

Make kedro project commands work from any directory inside a project #1831

Closed

ankatiyar added 4 commits March 5, 2024 14:22

Refactor fn

caf3eb0

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

Fix _find_kedro_project

b318f42

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

Make kedro commands available inside subdirectories

451477e

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

Add e2e test and update release notes

c39b2fd

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

ankatiyar changed the title ~~[Draft] Make kedro commands work from inside subdirectories in project~~ Make kedro commands work from inside subdirectories in project Mar 5, 2024

Fix e2e test

fccf145

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

ankatiyar marked this pull request as ready for review March 6, 2024 10:07

ankatiyar requested a review from merelcht as a code owner March 6, 2024 10:07

ankatiyar requested a review from noklam March 6, 2024 10:07

ankatiyar mentioned this pull request Mar 6, 2024

provide better error message outside of a kedro project #3680

Merged

7 tasks

noklam approved these changes Mar 6, 2024

View reviewed changes

noklam self-requested a review March 6, 2024 15:51

merelcht reviewed Mar 6, 2024

View reviewed changes

noklam mentioned this pull request Mar 6, 2024

Implementation of Directory Selection for Server Startup [Santana's PR] kedro-org/kedro-viz#1792

Closed

5 tasks

ankatiyar and others added 2 commits March 7, 2024 10:28

Resolve paths for session and update release notes

e5a9667

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>

Merge branch 'main' into kedro-commands-in-subdirectories

b261c99

Signed-off-by: Ankita Katiyar <110245118+ankatiyar@users.noreply.github.com>

ankatiyar requested a review from merelcht March 7, 2024 10:29

noklam approved these changes Mar 7, 2024

View reviewed changes

merelcht approved these changes Mar 7, 2024

View reviewed changes

ankatiyar merged commit a22bc32 into main Mar 7, 2024
34 checks passed

ankatiyar deleted the kedro-commands-in-subdirectories branch March 7, 2024 15:58

This was referenced Mar 8, 2024

refactor(airflow): Remove bootstrap_project kedro-org/kedro-plugins#599

Merged

feat(telemetry): Unique User IDs in kedro-telemetry - merge only for kedro-telemetry release 0.4.0 kedro-org/kedro-plugins#596

Merged

noklam mentioned this pull request Apr 26, 2024

Fix Kedro-Viz waiting for valid Kedro Project kedro-org/kedro-viz#1871

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make kedro commands work from inside subdirectories in project #3683

Make kedro commands work from inside subdirectories in project #3683

ankatiyar commented Mar 5, 2024 •

edited

Loading

datajoely commented Mar 6, 2024

noklam commented Mar 6, 2024

noklam left a comment •

edited

Loading

noklam commented Mar 6, 2024

ravi-kumar-pilla commented Mar 6, 2024

merelcht left a comment

merelcht Mar 6, 2024

ankatiyar Mar 7, 2024

merelcht Mar 6, 2024

ankatiyar Mar 7, 2024

noklam left a comment

merelcht left a comment

astrojuanlu commented Mar 7, 2024

noklam commented Mar 8, 2024

Make kedro commands work from inside subdirectories in project #3683

Make kedro commands work from inside subdirectories in project #3683

Conversation

ankatiyar commented Mar 5, 2024 • edited Loading

Description

Development notes

Developer Certificate of Origin

Checklist

datajoely commented Mar 6, 2024

noklam commented Mar 6, 2024

noklam left a comment • edited Loading

Choose a reason for hiding this comment

Out of scope:

noklam commented Mar 6, 2024

ravi-kumar-pilla commented Mar 6, 2024

merelcht left a comment

Choose a reason for hiding this comment

merelcht Mar 6, 2024

Choose a reason for hiding this comment

ankatiyar Mar 7, 2024

Choose a reason for hiding this comment

merelcht Mar 6, 2024

Choose a reason for hiding this comment

ankatiyar Mar 7, 2024

Choose a reason for hiding this comment

noklam left a comment

Choose a reason for hiding this comment

merelcht left a comment

Choose a reason for hiding this comment

astrojuanlu commented Mar 7, 2024

noklam commented Mar 8, 2024

ankatiyar commented Mar 5, 2024 •

edited

Loading

noklam left a comment •

edited

Loading