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

files.makedirs has no apparent effect #777

Closed drewp closed 2 years ago

drewp commented 2 years ago

bad:

    files.download(src=...     dest=dl)
    files.makedirs(local, exist_ok=True)  # this has no effect
    server.shell([
        f"aunpack --extract-to={local} {dl}",
    ])

--> Starting operation: Files/Download (src=https://github.com/kovidgoyal/kitty/releases/download/v0.24.4/kitty-0.24.4-x86_64.txz, dest=/tmp/kitty-0.24.4-x86_64.txz)
    [slash] No changes

--> Starting operation: Server/Shell (['aunpack --extract-to=/home/drewp/.local/kitty /tmp/kitty-0.24.4-x86_64.txz'])
    [slash] tar: /home/drewp/.local/kitty: Cannot open: No such file or directory
    [slash] tar: Error is not recoverable: exiting now
    [slash] aunpack: xz ... | tar ...: non-zero return-code
    [slash] Error

good:

    files.download(src=...     dest=dl)
    files.makedirs(local, exist_ok=True)
    server.shell([
        f"mkdir -p {local}",  # this workaround fixed it
        f"aunpack --extract-to={local} {dl}",
    ])

--> Starting operation: Files/Download (src=https://github.com/kovidgoyal/kitty/releases/download/v0.24.4/kitty-0.24.4-x86_64.txz, dest=/tmp/kitty-0.24.4-x86_64.txz)
    [slash] No changes

--> Starting operation: Server/Shell (['mkdir -p /home/drewp/.local/kitty', 'aunpack --extract-to=/home/drewp/.local/kitty /tmp/kitty-0.24.4-x86_64.txz'])
    [slash] Success

--> Support information:

If you are having issues with pyinfra or wish to make feature requests, please
check out the GitHub issues at https://github.com/Fizzadar/pyinfra/issues .
When adding an issue, be sure to include the following:

System: Linux
  Platform: Linux-5.13.0-30-generic-x86_64-with-glibc2.34
  Release: 5.13.0-30-generic
  Machine: x86_64
pyinfra: v1.7
Executable: env/bin/pyinfra
Python: 3.9.9 (CPython, GCC 10.3.0)
Fizzadar commented 2 years ago

Hi @drewp - was a bit confused by this, files.makedirs isn't an operation at all! It's actually just importing os.makdirs which is imported/used in the files.py module. Using the files.directory operation should enable you to achieve the desired result.

I suppose this is a weird gotcha with having everything be Python code. It would definitely be nice to avoid/prevent/warn about this somehow, but I'm not quite sure how (aside from prefixing them all or similar, which is not ideal!).

drewp commented 2 years ago

Ha, I see. I probably got it from autocomplete and assumed it must be an operation. Possible fixes:

Fizzadar commented 2 years ago

makedirs = directory at the bottom (kidding- don't do this)

🤣

Unfortunately __all__ doesn't prevent such imports, I'll change things to just import os and use os.makedirs etc!

drewp commented 2 years ago

Whoa, I never knew __all__ was only to control import *!

Your fix sgtm.

Fizzadar commented 2 years ago

Whoa, I never knew __all__ was only to control import *!

Me either, only discovered it looking into this!

v2 + 1.7.1 include the fix as above.