Refactor local runner and add `run` command #234

GeorgesLorre · 2023-06-23T21:56:30Z

No description provided.

RobbeSneyders

Can you explain a bit why you switched to this approach? One downside I see is that we're no longer building the parser hierarchy up front, which might not allow for cli autocompletion (to be checked).

RobbeSneyders · 2023-06-26T14:08:40Z

fondant/cli.py

+        """Check if the next command is help and print help if it is.
+        Otherwise parse the known args.
+        """
+        if input_args[0] in ["help", "-h", "--help"]:


Why don't we rely on the built-in behavior of argparse here?

The parse_known_args() will always consume the help. So if you do fondant compile local --help you will get the help page for just fondant because the first parse_know_args() will consume it.

RobbeSneyders · 2023-06-26T14:11:48Z

fondant/cli.py

+        parser = argparse.ArgumentParser(add_help=False)
+
+        parser.add_argument(
+            "mode",


I think it's a more logical user interface if this is a flag instead of a subcommand.

So:

fondant compile <pipeline> --local

instead of

fondant compile local <pipeline>

I think this can be achieved in the current implementation. We might just have to lift the pipeline parameter from compile_{mode} to this function, which I think makes sense.

RobbeSneyders · 2023-06-26T14:12:17Z

fondant/cli.py

+        parser = argparse.ArgumentParser(add_help=False)
+
+        parser.add_argument(
+            "mode",


Same comment here as above.

GeorgesLorre · 2023-06-26T14:26:15Z

Thanks @GeorgesLorre!

Can you explain a bit why you switched to this approach? One downside I see is that we're no longer building the parser hierarchy up front, which might not allow for cli autocompletion (to be checked).

Because of multiple subcommand levels fondant(1) compile(2) local(3) (see our chat). But we could change it back and build the hierarchy up front.

RobbeSneyders

I like the --local and --kubeflow flags instead of subcommands as the user interface. But can we now still define separate flags for each of them?

RobbeSneyders · 2023-06-27T07:25:43Z

fondant/cli.py

+def run(args):
+    if args.local:
+        try:
+            pipeline = pipeline_from_string(args.ref)


We only need the try except around this line I think.

I could move the logic around but I believe this is the clearest. Try to see if it is an importstring if it is then compile it otherwise assume it is a reference to an already compiled pipeline.

You can use

try: ... except: ... else: ... finally: ...

RobbeSneyders · 2023-06-27T07:26:36Z

fondant/cli.py

    sys.path.append(".")
-    args = cli.parse_args()
+
+    args = parser.parse_args(sys.argv[1:])


Suggested change

args = parser.parse_args(sys.argv[1:])

args = parser.parse_args()

I don't think explicitly providing the arguments is necessary.

ChristiaensBert · 2023-06-28T07:16:39Z

README.md

-We have compiled some references and created some scripts to [get you started](https://fondant.readthedocs.io/en/latest/infrastructure) with setting up the required infrastructure.
+There are 2 ways of using fondant:
+
+- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All fondant needs is an url pointing to the kubeflow pipeline host and a Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.


Suggested change

- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All fondant needs is an url pointing to the kubeflow pipeline host and a Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.

- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All that fondant needs is an url pointing to the kubeflow pipeline host and an Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.

ChristiaensBert · 2023-06-28T07:18:41Z

README.md

+- Or locally by using [docker compose](https://docs.docker.com/compose/). This way is mainly aimed at helping you develop fondant pipelines and components faster by making it easier to run things on a smaller scale.
+
+
+The same pipeline can be used in both variant allowing you to quickly develop and iterate using the local docker compose implementation and then using the power of kubeflow pipelines to run a large scale pipeline.


Suggested change

The same pipeline can be used in both variant allowing you to quickly develop and iterate using the local docker compose implementation and then using the power of kubeflow pipelines to run a large scale pipeline.

The same pipeline can be used in both variants, allowing you to quickly develop and iterate using the local docker compose implementation and then using the power of kubeflow pipelines to run a large scale pipeline.

ChristiaensBert · 2023-06-28T07:19:47Z

README.md

+
+###  Running your pipeline
+
+Once you have a pipeline you can easily run (and compile it) using the builtin CLI:


Suggested change

Once you have a pipeline you can easily run (and compile it) using the builtin CLI:

Once you have a pipeline you can easily run (and compile) it by using the built-in CLI:

ChristiaensBert · 2023-06-28T07:21:14Z

README.md

+Once you have a pipeline you can easily run (and compile it) using the builtin CLI:
+
+```bash
+fondant run foo/bar.py:pipeline --local


This is for the --local, should we add both?

the kubeflow command is there but raises NotImplementedError's

RobbeSneyders

Thanks @GeorgesLorre! 2 small comments.

RobbeSneyders · 2023-06-28T07:24:44Z

README.md

+Once you have a pipeline you can easily run (and compile it) using the builtin CLI:
+
+```bash
+fondant run foo/bar.py:pipeline --local


Suggested change

fondant run foo/bar.py:pipeline --local

fondant run foo.bar:pipeline --local

You should pass the file as a module, right?

RobbeSneyders · 2023-06-28T07:25:41Z

fondant/cli.py

+def run(args):
+    if args.local:
+        try:
+            pipeline = pipeline_from_string(args.ref)


NielsRogge · 2023-06-28T07:27:47Z

README.md

-We have compiled some references and created some scripts to [get you started](https://fondant.readthedocs.io/en/latest/infrastructure) with setting up the required infrastructure.
+There are 2 ways of using fondant:
+
+- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All fondant needs is an url pointing to the kubeflow pipeline host and a Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.


Suggested change

- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All fondant needs is an url pointing to the kubeflow pipeline host and a Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.

- Leveraging [Kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any Kubernetes cluster. All Fondant needs is a url pointing to the Kubeflow pipeline host and an Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.

NielsRogge · 2023-06-28T07:28:25Z

README.md

+
+- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All fondant needs is an url pointing to the kubeflow pipeline host and a Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.
+  We have compiled some references and created some scripts to [get you started](https://fondant.readthedocs.io/en/latest/infrastructure) with setting up the required infrastructure.
+- Or locally by using [docker compose](https://docs.docker.com/compose/). This way is mainly aimed at helping you develop fondant pipelines and components faster by making it easier to run things on a smaller scale.


Suggested change

- Or locally by using [docker compose](https://docs.docker.com/compose/). This way is mainly aimed at helping you develop fondant pipelines and components faster by making it easier to run things on a smaller scale.

- Or locally by using [Docker Compose](https://docs.docker.com/compose/). This way is mainly aimed at helping you develop Fondant pipelines and components faster by making it easier to run things on a smaller scale.

NielsRogge · 2023-06-28T07:28:53Z

README.md

+- Or locally by using [docker compose](https://docs.docker.com/compose/). This way is mainly aimed at helping you develop fondant pipelines and components faster by making it easier to run things on a smaller scale.
+
+
+The same pipeline can be used in both variant allowing you to quickly develop and iterate using the local docker compose implementation and then using the power of kubeflow pipelines to run a large scale pipeline.


Suggested change

The same pipeline can be used in both variant allowing you to quickly develop and iterate using the local docker compose implementation and then using the power of kubeflow pipelines to run a large scale pipeline.

The same pipeline can be used in both variant allowing you to quickly develop and iterate using the local Docker Compose implementation and then using the power of Kubeflow pipelines to run a large scale pipeline.

RobbeSneyders · 2023-06-28T09:41:16Z

README.md

+Once you have a pipeline you can easily run (and compile) it by using the built-in CLI:
+
+```bash
+fondant run foo.bar.py:pipeline --local


Suggested change

fondant run foo.bar.py:pipeline --local

fondant run foo.bar:pipeline --local

The .py should also be removed.

RobbeSneyders · 2023-06-28T09:41:48Z

fondant/runner.py

+        """Run a docker-compose spec."""
+        cmd = ["docker", "compose", "-f", input_spec, "up", "--build"]
+
+        subprocess.call(cmd, stdout=subprocess.PIPE)  # nosec


Suggested change

subprocess.call(cmd, stdout=subprocess.PIPE) # nosec

subprocess.call(cmd) # nosec

As discussed, this intercepts the build logs and docker compose log colors.

GeorgesLorre added 2 commits June 23, 2023 23:54

Add dynamic fondant cli and local runner

33c159a

Add dynamic fondant cli and local runner

207173b

GeorgesLorre requested review from RobbeSneyders, NielsRogge and ChristiaensBert June 23, 2023 21:56

RobbeSneyders reviewed Jun 26, 2023

View reviewed changes

Simplify CLI

b5e2962

RobbeSneyders reviewed Jun 27, 2023

View reviewed changes

GeorgesLorre added 2 commits June 27, 2023 14:11

Add descriptions to cli

0537aab

Update readme

f205d9d

ChristiaensBert reviewed Jun 28, 2023

View reviewed changes

RobbeSneyders approved these changes Jun 28, 2023

View reviewed changes

NielsRogge reviewed Jun 28, 2023

View reviewed changes

Include documentation suggestions

e5f7569

RobbeSneyders changed the title ~~Feature/local runner~~ Refactor local runner and add run command Jun 28, 2023

RobbeSneyders reviewed Jun 28, 2023

View reviewed changes

Fix runner output

756c4db

GeorgesLorre merged commit 01cb057 into main Jun 28, 2023

GeorgesLorre deleted the feature/local-runner branch June 28, 2023 10:05

Hakimovich99 pushed a commit that referenced this pull request Oct 16, 2023

Refactor local runner and add run command (#234)

5a5149c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor local runner and add `run` command #234

Refactor local runner and add `run` command #234

GeorgesLorre commented Jun 23, 2023

RobbeSneyders left a comment

RobbeSneyders Jun 26, 2023

GeorgesLorre Jun 26, 2023 •

edited

Loading

RobbeSneyders Jun 26, 2023

RobbeSneyders Jun 26, 2023

GeorgesLorre commented Jun 26, 2023

RobbeSneyders left a comment

RobbeSneyders Jun 27, 2023

RobbeSneyders Jun 28, 2023

GeorgesLorre Jun 28, 2023

RobbeSneyders Jun 28, 2023

RobbeSneyders Jun 27, 2023

ChristiaensBert Jun 28, 2023

ChristiaensBert Jun 28, 2023

ChristiaensBert Jun 28, 2023

ChristiaensBert Jun 28, 2023

GeorgesLorre Jun 28, 2023

RobbeSneyders left a comment

RobbeSneyders Jun 28, 2023

RobbeSneyders Jun 28, 2023

NielsRogge Jun 28, 2023

NielsRogge Jun 28, 2023

NielsRogge Jun 28, 2023

RobbeSneyders Jun 28, 2023

RobbeSneyders Jun 28, 2023

	args = parser.parse_args(sys.argv[1:])
	args = parser.parse_args()

	- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All fondant needs is an url pointing to the kubeflow pipeline host and a Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.
	- Leveraging [kubeflow pipelines](https://www.kubeflow.org/docs/components/pipelines/v1/introduction/) on any kubernetes cluster. All that fondant needs is an url pointing to the kubeflow pipeline host and an Object Storage provider (S3, GCS, etc) to store data produced in the pipeline between steps.

		- Or locally by using [docker compose](https://docs.docker.com/compose/). This way is mainly aimed at helping you develop fondant pipelines and components faster by making it easier to run things on a smaller scale.


		The same pipeline can be used in both variant allowing you to quickly develop and iterate using the local docker compose implementation and then using the power of kubeflow pipelines to run a large scale pipeline.


		### Running your pipeline

		Once you have a pipeline you can easily run (and compile it) using the builtin CLI:

	fondant run foo/bar.py:pipeline --local
	fondant run foo.bar:pipeline --local

	fondant run foo.bar.py:pipeline --local
	fondant run foo.bar:pipeline --local

	subprocess.call(cmd, stdout=subprocess.PIPE) # nosec
	subprocess.call(cmd) # nosec

Refactor local runner and add run command #234

Refactor local runner and add run command #234

Conversation

GeorgesLorre commented Jun 23, 2023

RobbeSneyders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GeorgesLorre Jun 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GeorgesLorre commented Jun 26, 2023

RobbeSneyders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobbeSneyders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Refactor local runner and add `run` command #234

Refactor local runner and add `run` command #234

GeorgesLorre Jun 26, 2023 •

edited

Loading