Perhaps the trigger word can use some simpler equivalent:

• toolforge build run
• toolforge build create
• toolforge build add

In T322286#8365727, @aborrero wrote:

Perhaps the trigger word can use some simpler equivalent:

• toolforge build run
• toolforge build create
• toolforge build add

Agree a simpler name would be better. My vote goes to toolforge build create. The other subcommands look good to me.

It's just a feeling but toolforge build run looks better

+1 for build run

All having a --verbose option for debugging.

Do we want this flag to be associated with the main entry point, or with each subcommand (at any level of nesting)?

$ toolforge --verbose build run
$ toolforge build --verbose run
$ toolforge build run --verbose

Option 3 feels more natural to me, but option 1 is a bit cleaner to implement (just pass the context from the entry point down to the nested commands). Option 2 looks (to me) like the worst of both worlds.

Opinions?

In T322286#8401708, @Slst2020 wrote:

Opinions?

I would go with either option 1 or 3.

Ideally, I would like for both option 1 and option 3 to work. Otherwise, either one of those is fine, I agree that option 2 is confusing.

Another thing to keep in mind is that for external commands (binaries found in the $PATH, like toolforge-jobs), you can't really pass down the context with the verbose bit, and each might implement that flag differently (--debug, -vvv, ...) so well, we might end up with things like "toolforge --verbose build --verbose run --verbose".

The way git works, is that it does not have a verbose/debug flag on the main command, but enables debugging with env vars, and the rest of discovered subcommands each implement it however they prefer.

I think that we might want to do so for all, as in, have a --verbose flag in the parent command (option 1), but allow setting it with something like DEBUG=1, and what this flag does, is it also sets the env var if not there when calling the subcommand so any external subcommand can pick it up if implemented there.

Though that might be a second iteration of this, and just go with option 1 for now and pass down the flag to the build subcommand.

So my opinion is:
Go with option 1, and if you feel willing to spend a bit more time, implement the DEBUG env var for it, if not, let me know and I'll create a task for it to pick up later (or feel free to do so yourself if you prefer).

I know this is a discussion we've had already, but now that I've started work on this, I'd like us to reconsider the renaming toolforge build -> toolforge build run

This command builds a container image from source, and pushes it to our container image registry. It does "run" a pipeline, but this is an implementation detail that, in my opinion, should be hidden from the end user.

Additionally, "running" a project is often associated with getting it deployed, which is not what is happening here. Maybe toolforge build prepare would more accurately convey what's actually going on?

In T322286#8405489, @Slst2020 wrote:

I know this is a discussion we've had already, but now that I've started work on this, I'd like us to reconsider the renaming toolforge build -> toolforge build run

This command builds a container image from source, and pushes it to our container image registry. It does "run" a pipeline, but this is an implementation detail that, in my opinion, should be hidden from the end user.

Additionally, "running" a project is often associated with getting it deployed, which is not what is happening here. Maybe toolforge build prepare would more accurately convey what's actually going on?

+1

In T322286#8405489, @Slst2020 wrote:

I know this is a discussion we've had already, but now that I've started work on this, I'd like us to reconsider the renaming toolforge build -> toolforge build run

This command builds a container image from source, and pushes it to our container image registry. It does "run" a pipeline, but this is an implementation detail that, in my opinion, should be hidden from the end user.

Additionally, "running" a project is often associated with getting it deployed, which is not what is happening here. Maybe toolforge build prepare would more accurately convey what's actually going on?

Agree that run can be confused, that's one of the advantages of trigger, as that's the technical term (as in triggered by a push to a git repository, or from the cli).

prepare seems misleading to me as in I would expect it to not do anything (build an image, or push it).

Maybe start is better? As in 'toolforge build start'?

In T322286#8406177, @dcaro wrote:

Maybe start is better? As in 'toolforge build start'?

I prefer start to run, although semantically, I think both convey that something continues to run, i.e. the "thing" that was started keeps running until it's stopped.

Agree that run can be confused, that's one of the advantages of trigger, as that's the technical term (as in triggered by a push to a git repository, or from the cli).

Maybe build push then? :) Or back to build create?

In T322286#8406245, @Slst2020 wrote:

In T322286#8406177, @dcaro wrote:

Maybe start is better? As in 'toolforge build start'?

I prefer start to run, although semantically, I think both convey that something continues to run, i.e. the "thing" that was started keeps running until it's stopped.

Might be what you mean, but just to clarify, when you run the cli, it will start a process (as in trigger a cascade of events, not os process) that will run in the background asynchronously, no matter what you do with the client (crtl-c, kill, logout, ...), so the client is not running anything itself.

Maybe build push then? :) Or back to build create?

I think that push and create have the same issues, the built image (if it succeeds) will be created and pushed way after the client has finished running, as the process is asynchronous (and most probably running in a different host).
Essentially running the client from start to finish will not push nor create anything for the user.

Anyhow, I think start is a good compromise between trigger and run that leaves little room for misunderstandings like create or push could.

Ok, let's go with start for now

In T322286#8401737, @fnegri wrote:

Ideally, I would like for both option 1 and option 3 to work. Otherwise, either one of those is fine, I agree that option 2 is confusing.

This is what I've ended up implementing. Option 1 is what click defaults to when a command is defined on a group. Option 3 is achieved by creating a custom add_options decorator, and calling it with the options that are shared between all subcommands of toolforge build, i.e. --verbose and --kubeconfig. This does not pass down any flags to external plugins (a.k.a. discovered subcommands).

In T322286#8401802, @dcaro wrote:

The way git works, is that it does not have a verbose/debug flag on the main command, but enables debugging with env vars, and the rest of discovered subcommands each implement it however they prefer.

I think that we might want to do so for all, as in, have a --verbose flag in the parent command (option 1), but allow setting it with something like DEBUG=1, and what this flag does, is it also sets the env var if not there when calling the subcommand so any external subcommand can pick it up if implemented there.

Though that might be a second iteration of this, and just go with option 1 for now and pass down the flag to the build subcommand.

So my opinion is:
Go with option 1, and if you feel willing to spend a bit more time, implement the DEBUG env var for it, if not, let me know and I'll create a task for it to pick up later (or feel free to do so yourself if you prefer).

This sounds good to me. I will create a separate task for it.

Change 859439 had a related patch set uploaded (by Slavina Stefanova; author: Slavina Stefanova):

[cloud/toolforge/toolforge-cli@main] cli: Create proper subcommands

https://gerrit.wikimedia.org/r/859439

Change 859439 merged by jenkins-bot:

[cloud/toolforge/toolforge-cli@main] cli: Create nested subcommands

https://gerrit.wikimedia.org/r/859439