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

Future planning: how to manage global arguments #695

Closed Fizzadar closed 2 years ago

Fizzadar commented 2 years ago

pyinfra has a number of global @operation/@deploy arguments, these are "magic" in that they are extracted before other arguments are passed into the operation/deploy function. This means that these functions cannot use arguments with the same name as global arguments.

While this works OK for builtin operations, where this issue can be managed, it's a disaster for 3rd party operations and deploys. It effectively bocks additional global arugments being added (such as docker_user, which would be very useful).

# Currently:
files.file(
    name='Create a file as the pyinfra user using sudo',
    path='/home/pyinfra/sudo_testfile',
    sudo_user='pyinfra',
    env={'PATH': '/home/pyinfra/.local/bin'},
)

# Could become:
with operation_args(sudo_user='pyinfra', env={'PATH': '/home/pyinfra/.local/bin'}):
    files.file(
        name='Create a file as the pyinfra user using sudo',
        path='/home/pyinfra/sudo_testfile',
    )

# OR, make a simple rule that _ prefixed commands are reserved:
files.file(
    name='Create a file as the pyinfra user using sudo',
    path='/home/pyinfra/sudo_testfile',
    _sudo_user='pyinfra',
    _env={'PATH': '/home/pyinfra/.local/bin'},
)

# OR, a dictionary for all such commands
files.file(
    name='Create a file as the pyinfra user using sudo',
    path='/home/pyinfra/sudo_testfile',
    operation_args={
        'sudo_user': 'pyinfra',
        'env': {'PATH': '/home/pyinfra/.local/bin'},
    },
)

# OR, nicer(?) style
files.file(
    name='Create a file as the pyinfra user using sudo',
    path='/home/pyinfra/sudo_testfile',
    operation_args=dict(
        sudo_user='pyinfra',
        env={'PATH': '/home/pyinfra/.local/bin'},
    ),
)

Currently leaning towards the dict (option 3+4), with the final option style wise, only because it's easier to read (IMO, very subjective) and is the closest match to the current way.

Fizzadar commented 2 years ago

From: https://github.com/Fizzadar/pyinfra/issues/585#issuecomment-860271266

This might be the right point to also start supporting Solaris'/Illumos' pfexec, and maybe unify the interface of using them? Instead of having various use_login, usepassword, preserve_env, remove the specific name, and provide a new parameter like e.g. priv_tool that can be set to one of the supported methods. Then I could write in my deploys files.file(..., priv_tool=doas, use_priv_login=True).

This way, when I adapt the deploy to another OS, or simply when I have to switch to sudo because I started to need to use some of its features, I only have change it at one point. That point could even be somewhere central, instead of in every single operation.

Gaming4LifeDE commented 2 years ago

I'd prefer option 3 I think just because option 4 is more verbose where it doesn't need to be. On the other hand, another solution could be just to prefix like Ansible does (e.g. pyinfra_sudo_user)

Fizzadar commented 2 years ago

_ prefixed gloal arguments is now part of v2, with backwards compatibility until v3.

Fizzadar commented 2 years ago

Released in v2!