Skip to content

feat: add model cli commands#282

Open
MatteoPologruto wants to merge 7 commits intomainfrom
add-models-cli-commands
Open

feat: add model cli commands#282
MatteoPologruto wants to merge 7 commits intomainfrom
add-models-cli-commands

Conversation

@MatteoPologruto
Copy link
Contributor

@MatteoPologruto MatteoPologruto commented Mar 4, 2026

Motivation

Add model cli commands, following-up to the model api interface.

Change description

Added the following cli commands:

  • model list [--exclude-builtin]
  • model delete <id> [--force]

Additional Notes

Reviewer checklist

  • PR addresses a single concern.
  • PR title and description are properly filled.
  • Changes will be merged in main.
  • Changes are covered by tests.
  • Logging is meaningful in case of troubleshooting.

@MatteoPologruto MatteoPologruto self-assigned this Mar 4, 2026
@MatteoPologruto MatteoPologruto added the enhancement New feature or request label Mar 4, 2026
@MatteoPologruto MatteoPologruto marked this pull request as ready for review March 5, 2026 08:33
@MatteoPologruto MatteoPologruto requested a review from a team March 5, 2026 08:33
ModelID string `json:"model_id"`
}

func (r deleteModelResult) String() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the mode cannot be deleted since is in use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the --force flag is not used, it should fail with feedback.Fatal(err.Error(), feedback.ErrGeneric).

cmd := &cobra.Command{
Use: "delete",
Short: "Delete the provided model",
Args: cobra.ExactArgs(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could accept more than one model name in order to delete them all in action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usually we only accept one parameter when performing this kind of operations in the various CLIs we have, but I don't have a strong opinion on this matter.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we support multiple models, we need to define the behavior when one model can be deleted but another cannot. Should the deletion be 'all or nothing' (atomic) or 'best effort'? Unless required otherwise, I will keep it simple.

Co-authored-by: Luca Rinaldi <l.rinaldi@arduino.cc>
@MatteoPologruto MatteoPologruto requested a review from dido18 March 11, 2026 08:47
func (r modelListResult) String() string {
t := table.NewWriter()
t.SetStyle(tablestyle.CustomCleanStyle)
t.AppendHeader(table.Row{"ID", "NAME", "RUNNER", "BUILTIN"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I would avoid the runner that is always "brick"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants