pyinfra-dev / pyinfra

pyinfra turns Python code into shell commands and runs them on your servers. Execute ad-hoc commands and write declarative operations. Target SSH servers, local machine and Docker containers. Fast and scales from one server to thousands.
https://pyinfra.com
MIT License
3.85k stars 374 forks source link

`host.run_shell_command` is used in examples, but is undocumented (ditto for nearly all methods of `host.Host`) #853

Closed phlummox closed 1 year ago

phlummox commented 2 years ago

Describe the bug

The host.run_shell_command() method is used in examples – for instance, the "Dynamic Execution during Deploy" example, here:

https://github.com/Fizzadar/pyinfra/blob/3397c5b90743e7560fc76c5c36d18a16fc2b88c4/docs/examples/dynamic_execution_deploy.md?plain=1#L20

– but is undocumented; it (together with most of the methods of the Host class) has no Python docstring, nor any explanation on https://docs.pyinfra.com of what it does and what it returns. If the method is useful enough to turn up in examples, it probably should be properly documented. (Having docstrings for the methods would also be useful for developers trying to get information on them from the Python REPL.)

It would also be useful to explain (either in the docstring documentation, or in the "Dynamic Execution during Deploy" example) in what ways run_shell_command differs from the server.shell operation.

(I am guessing that this method, together with put_file and get_file, effectively acts as "back door" access to the host, allowing information to be extracted from the host, or its state altered, "outside" the operations paradigm – getting around some of the control-flow limitations users would otherwise encounter.)

To Reproduce

View the code for the Host class at https://github.com/Fizzadar/pyinfra/blob/3397c5b90743e7560fc76c5c36d18a16fc2b88c4/pyinfra/api/host.py#L74, and note that run_shell_command (and most other methods) are undocumented.

Expected behavior

Some sort of documentation for the method should be available.

Actual behavior

No documentation for the method is available.

Meta

Fizzadar commented 2 years ago

Agreed this should be documented at least with docstrings. The addition of nested operations in 2.1 should reduce the need for calling these methods, but I still think this low-level access is handy to have just in case.

Fizzadar commented 1 year ago

I've now added docstrings to all the Host.* methods in https://github.com/Fizzadar/pyinfra/commit/601f39550d0e8cbfff58a08f6567957dd76c17c4.

phlummox commented 1 year ago

Many thanks! I'll check out the documentation. Cheers :)