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

Basic conduit init and conduit pipelines init commands #1927

Merged
merged 47 commits into from
Nov 6, 2024

Conversation

hariso
Copy link
Contributor

@hariso hariso commented Oct 24, 2024

Description

Closes #1924.

High-level overview of changes:

  • There's a new package cli with the CLI-related code.
  • main.go now invokes the CLI.
  • The CLI adds 2 new commands, init and pipelines, and one sub-command, pipelines init.
  • Running the server has been moved to the root command. The existing flags for Conduit have been moved to the root command.
  • The default value of --pipelines.path in conduit pipelines init is now ./pipelines. This makes it easier to get started with Conduit (conduit init && conduit pipelines init, without specifying the path). IMO, it makes sense also because the pipelines' directory is relative to the binary/working directory.

Tested commands:

./conduit

./conduit --http.address :8888

./conduit init

./conduit init --config.path /tmp/conduit
tree /tmp/conduit

./conduit pipelines init
cat pipeline-*yaml

./conduit pipelines init --source postgres
cat pipeline-*yaml

./conduit pipelines init --destination kafka
cat pipeline-*yaml

./conduit pipelines init my-pipeline --source postgres --destination kafka --pipelines.path /tmp/conduit-pipelines/
cat /tmp/conduit-pipelines/pipeline-my-pipeline.yaml

./conduit help

./conduit --version

./conduit --invalid

Quick checks

  • I have followed the Code Guidelines.
  • There is no other pull request for the same update/change.
  • I have written unit tests.
  • I have made sure that the PR is of reasonable size and can be easily reviewed.

@hariso hariso marked this pull request as ready for review October 24, 2024 11:50
@hariso hariso requested a review from a team as a code owner October 24, 2024 11:50
@raulb
Copy link
Member

raulb commented Oct 24, 2024

@hariso I think init is ypically used to set up the initial structure, configuration, and dependencies for a new project or application. Here are some industry examples:

  • npm init The npm init command is used to set up a new or existing npm package
  • tsc --init Initializes a TypeScript project and creates a tsconfig.json file.
  • git init: Initializes a new Git repository in the current directory, creating the necessary files and directories to start version controlling your project.
  • webpack init: Helps you set up a new Webpack configuration by asking you a series of questions and generating a basic Webpack configuration file (webpack.config.js).
  • vue init: Generates a new Vue.js project based on a specific template. It prompts the user with questions to customize the project setup.
  • terraform init: Initializes a new Terraform working directory, creating the necessary files and directories to start managing infrastructure as code.
  • yarn init: Initializes a new Yarn project, creating a package.json file and setting up the initial project structure.
  • kubectl init: Initializes a new Kubernetes cluster, setting up the necessary components and configuration files.

By having --build-pipeline I also see the disadvantage of not distinguishing the command with flags. --build-pipeline as it is now looks like a global flag for the conduit CLI.

For that reason, I feel strongly opinionated that the command needs to be init and nothing else.

Update: One additional option is to have conduit init be an alias for conduit pipelines init since that's what we're technically doing.

@nickchomey
Copy link

nickchomey commented Oct 24, 2024

FWIW, I agree with raul.

Also "build" suggests that something is actually building - eg. go build. If I understand this correctly, init is just setting up config files etc - as is done in every other tool.

@hariso
Copy link
Contributor Author

hariso commented Oct 24, 2024

We discussed this at our daily sync, here's the summary

  • conduit init will initialize a "working directory" for Conduit, i.e. create connectors, processors and pipelines directory, and the conduit.yaml config gile
  • once conduit init is done, it will inform the user to run conduit pipelines init, to initialize a pipeline.
  • conduit pipelines init does what we already mentioned.

@nickchomey
Copy link

Not that my opinion should matter much here, but I'm curious why you dont just have the one command do both things?

@raulb
Copy link
Member

raulb commented Oct 24, 2024

We discussed this at our daily sync, here's the summary

  • conduit init will initialize a "working directory" for Conduit, i.e. create connectors, processors and pipelines directory, and the conduit.yaml config gile
  • once conduit init is done, it will inform the user to run conduit pipelines init, to initialize a pipeline.
  • conduit pipelines init does what we already mentioned.

Yes, I was convinced today. There’s something that wasn’t mentioned above, which hopefully addresses @nickchomey’s latest comment.

The idea behind conduit init is that it will accept an optional flag --path, with the current directory as its default value. This will allow users to initialize different environments if needed.

While we don’t expect conduit init to be run more than once, conduit pipelines init is a command that could (and likely will) be executed multiple times. Therefore, these two actions should remain separate.

@raulb
Copy link
Member

raulb commented Oct 24, 2024

I forgot to mention something above. The intention behind the conduit init command includes an educational component. This command will initialize a working conduit environment, demonstrating what it comprises (it's more than just a pipeline YAML file). We believe this is helpful for illustrating the different components of a conduit project.

pkg/cli/conduit_init.go Outdated Show resolved Hide resolved
pkg/cli/cli.go Outdated Show resolved Hide resolved
@hariso
Copy link
Contributor Author

hariso commented Oct 29, 2024

Not that my opinion should matter much here, but I'm curious why you dont just have the one command do both things?

Opinions of users always matter.:)

Can you elaborate a bit more on "one command doing both things"?

@hariso hariso enabled auto-merge (squash) October 29, 2024 14:15
cmd/cli/pipelines_init.go Outdated Show resolved Hide resolved
Copy link
Member

@raulb raulb left a comment

Choose a reason for hiding this comment

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

@hariso nice work!

Only one thing I'd like to add before we ship this. Looks like there's no output when running pipelines init. I think we should do something similar like you did on conduit init

Created directory: /Users/rb/code/conduitio/conduit/processors
Directory '/Users/rb/code/conduitio/conduit/connectors' already exists, skipping...
Directory '/Users/rb/code/conduitio/conduit/pipelines' already exists, skipping...
Configuration file written to /Users/rb/code/conduitio/conduit/conduit.yaml

Conduit has been initialized!

To quickly create an example pipeline, run 'conduit pipelines init'.
To see how you can customize your first pipeline, run 'conduit pipelines init --help'.

Copy link
Member

@raulb raulb left a comment

Choose a reason for hiding this comment

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

@hariso only one suggestion. Otherwise, looks good to me!

cmd/cli/pipelines_init.go Outdated Show resolved Hide resolved
Co-authored-by: Raúl Barroso <ra.barroso@gmail.com>
@hariso hariso merged commit a60c36a into main Nov 6, 2024
3 checks passed
@hariso hariso deleted the haris/conduit-init-simpler branch November 6, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A basic conduit init command
3 participants