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.91k stars 382 forks source link

pip.venv and pip.packages - should package names be lowercased? #927

Open sebastianelsner opened 1 year ago

sebastianelsner commented 1 year ago

Describe the bug

Please consider the following code snippet. When run multiple times, installing the packages via pip.packages is reported as "changed" every run.

To Reproduce

from pyinfra.api import deploy
from pyinfra.operations import pip

@deploy("Venv")
def venv():
    pip.venv(
        python="python3",
        path="/tmp/clientvenv",
    )

    pip.packages(
        name="Install packages",
        packages=["boto3==1.26.27", "lucidity==1.6.0"],
        virtualenv="/tmp/clientvenv",
    )

venv()

Run via

pyinfra inventory_client.py deploy_client.py

All involved systems are Ubuntu 22.04.

Expected behavior

The packages are found in the given venv and not reported as changed every time. Please tell me I am just not seeing something basic here... I feel dumb, this should be easy :D

Meta

    System: Linux
      Platform: Linux-5.15.0-56-generic-x86_64-with-glibc2.35
      Release: 5.15.0-56-generic
      Machine: x86_64
    pyinfra: v2.5.2
    Executable: /home/grndgdnke/lidl_render_backend/lidl_render_backend_venv/bin/pyinfra
    Python: 3.10.6 (CPython, GCC 11.3.0)

venv + pip install

sysadmin75 commented 1 year ago

The reason this operation keeps showing as changed is because the package name in pypi is actually Lucidity.

Your deployment is using a lowercase version of the name so it's always returning that it needs to be installed.

The quick fix here is to change your deployment to use the actual package name:

    pip.packages(
        name="Install packages",
        packages=["boto3==1.26.27", "Lucidity==1.6.0"],
        virtualenv="/tmp/clientvenv",
    )

Whether or not this is a bug, or something that should be fixed is another discussion.

sebastianelsner commented 1 year ago

Oh good catch! Thank you.

Fizzadar commented 1 year ago

So pyinfra did used to, depending on package manager, lowercase package names. However this behaviour was confusing and resulted in other bugs. That said PyPi's own pep-0426 clearly states package names should be compared insensitively, this would probably require lowercasing input names and fact names...