pydantic / pydantic-settings

Settings management using pydantic
https://docs.pydantic.dev/latest/usage/pydantic_settings/
MIT License
555 stars 55 forks source link

Add get_subcommand function. #341

Closed kschwab closed 1 week ago

kschwab commented 1 month ago

Fixes #335

kschwab commented 1 month ago

@hramezani this PR is ready. Updated docs are here.

hramezani commented 1 month ago

@kschwab Thanks. I will take a look later this week.

hramezani commented 3 weeks ago

@kschwab Thanks for creating this PR.

I checked it a little bit and didn't like the idea of adding def main or def sub_main. isn't it possible to somehow remove these methods? can't the CLI settings source handle it(I mean calling the sub_command method)?

I got confused of the way that we should use the sub_command method. it is probably because:

Also, I am afraid that this introduces a breaking change and this is the thing that we hardly try to avoid

kschwab commented 3 weeks ago

@hramezani, I'll get back to these soon. No worries though on this PR with respect to breaking changes etc. It's purely a utility function. I'll update the docs to make it clearer. All it does is grab a subcommand if set, and raise an error if required:

class Git(BaseSettings, cli_parse_args=True, cli_prog_name='git'):
    """git - The stupid content tracker"""

    clone: CliSubCommand[Clone] = Field(description='Clone a repo ...')
    plugins: CliSubCommand[Plugins] = Field(description='Fake GIT plugins')

sys.argv = ['example.py', 'clone', 'repo', 'dest']
git = Git()
print(git.model_dump())
#> {'clone': {'repository': 'repo', 'directory': 'dest', 'local': False}, 'plugins': None}

print(get_subcommand(git).model_dump())
#> {'repository': 'repo', 'directory': 'dest', 'local': False}

sys.argv = ['example.py']
git = Git()
print(git.model_dump())
#> {'clone': None, 'plugins': None}

print(get_subcommand(git).model_dump())
#> Error: CLI subcomand is required {clone, plugins}

print(get_subcommand(git, is_required=False).model_dump())
#> None

The docs currently just show how it is most likely to be applied.

kschwab commented 1 week ago

Updated docs can be found here. Hopefully this is clearer @hramezani, let me know if we still need to adjust.

hramezani commented 1 week ago

@kschwab It is still unclear why we need the get_subcommand utility. why can't we use the .model_dump?

kschwab commented 1 week ago

@kschwab It is still unclear why we need the get_subcommand utility. why can't we use the .model_dump?

It's for convenience. Users can use model_dump etc., but it's atypical from a CLI application point of view for users to handle this manually.

e.g., if we look at docker subcommands:

$ docker help

Usage:  docker [OPTIONS] COMMAND

A self-sufficient runtime for containers

Common Commands:
  run         Create and run a new container from an image
  exec        Execute a command in a running container
  ps          List containers
  build       Build an image from a Dockerfile
  pull        Download an image from a registry
  push        Upload an image to a registry
  images      List images
  login       Log in to a registry
  logout      Log out from a registry
  search      Search Docker Hub for images
  version     Show the Docker version information
  info        Display system-wide information

Management Commands:
  builder     Manage builds
  container   Manage containers
  context     Manage contexts
  image       Manage images
  manifest    Manage Docker image manifests and manifest lists
  network     Manage networks
  plugin      Manage plugins
  system      Manage Docker
  trust       Manage trust on Docker images
  volume      Manage volumes

Swarm Commands:
  swarm       Manage Swarm

Commands:
  attach      Attach local standard input, output, and error streams to a running container
  commit      Create a new image from a container's changes
  cp          Copy files/folders between a container and the local filesystem
  create      Create a new container
  diff        Inspect changes to files or directories on a container's filesystem
  events      Get real time events from the server
  export      Export a container's filesystem as a tar archive
  history     Show the history of an image
  import      Import the contents from a tarball to create a filesystem image
  inspect     Return low-level information on Docker objects
  kill        Kill one or more running containers
  load        Load an image from a tar archive or STDIN
  logs        Fetch the logs of a container
  pause       Pause all processes within one or more containers
  port        List port mappings or a specific mapping for the container
  rename      Rename a container
  restart     Restart one or more containers
  rm          Remove one or more containers
  rmi         Remove one or more images
  save        Save one or more images to a tar archive (streamed to STDOUT by default)
  start       Start one or more stopped containers
  stats       Display a live stream of container(s) resource usage statistics
  stop        Stop one or more running containers
  tag         Create a tag TARGET_IMAGE that refers to SOURCE_IMAGE
  top         Display the running processes of a container
  unpause     Unpause all processes within one or more containers
  update      Update configuration of one or more containers
  wait        Block until one or more containers stop, then print their exit codes

Without get_subcommand, it means each user will have to implement their own function (using model_dump or some other means) to find the subcommand that was invoked. This is what was raised in #335 as an issue. The get_subcommand utility simply finds and returns the active subcommand so the user doesn't have to.

As an aside, outside of this PR, there are only two other features (union/alias support for CliSubCommand and CliApp) I believe are necessary for bringing the CliSettingsSource up to a typical CLI implementation. So not much more, but these last few I do think are important as they are relatively "standard" features for CLI applications.

hramezani commented 1 week ago

Thanks @kschwab for the explanation and for working on this.

Please resolve the conflict and then it will be ready to go

kschwab commented 1 week ago

Thanks @hramezani, good to go.