Improve usability of rez_pip as python library.

[MrLixm]

Warning

Draft pull request, check back when its ready !

Hello,
I don't know if it was in the goal of rez_pip to be usable as a python library and not only as a CLI, but given its current "WIP" state I found myself in need of such behaviour so I could further custom rez_pip behaviour.

The goal is to have rez_pip part of a larger python script, doing the heavy work of installing pip package, but allowing to retrieve those for further manipulations.

changes

⭐ Extract the cli._run private function to a new main module, as run_full_installation function.

• benefits:
  • indirectly allow to clearly defined the "inputs" of the function that were hidden being the args variable from argparse before
  • improve readability by using parameters with more complex names than ones defined in the CLI.
    examples: pip > pipPath, packages > pipPackageNames
• remarks:
  • are you satisfied with the name chosen for the new module + new function ?

⭐ Split run_full_installation into an additional lower-level function run_installation_for_python

• benefits:
  • offer a workaround for Failling with custom versioning for python package. #46
  • slightly increase readability by making run_full_installation lighter
  • are you satisfied with the name chosen for the new function ?

have rez.createPackage return the rez.package_maker.PackageMaker object created

• benefits:
  • allow to further manipulate the package created if needed, or retrieve additional information from it

have new run_full_installation function return a dict of rez package created for each python version, dependent on previous change.

• benefits:
  • fit in the goal of allowing rez_pip to be part of a larger script that could use that ouput for further refinement.

⭐ add a creationCallback parameter to rez.createPackage and main.run_full_installation (in f7b6519)

• benefits:
  • allow to apply specific override per package when using rez_pip as a python library. This could help address some pitfalls of packaging pip packages to rez.
• remarks:
  • the 3 arguments provided to the callable were arbitrarily chosen based on their probability of use. Don't hesitate to comment if you think more or less should be provided.

extract 2 functions from rez.getPythonExecutables: getPythonExecutable and findPythonPackages

• benefits:
  • improve readability by having smaller functions overall
  • make it easier to re-use specific behavior when using rez_pip as library
• remarks:
  • do you think the change is pertinent ?

change some variable to be pathlib.Path objects instead of str (in 13b7ee4 and a184ebf)

• benefits:
  • improve readability by clearly stating that we are expecting filesystem paths and not just strings.
  • easier to use path related function on them (no need for os.path)
• remarks:
  • do you think the change is pertinent ?

what to review

Not all those changes are necessary for the initial need. Do not hesitate to oppose to some of them if you think they are not pertinent.

Essential changes are marked with a ⭐ emoji.

notes

I intended commit history to be pretty clean but I actually did some change I choose to revert later for maintaining simplicity.

pitfalls

After a few test it seems rez_pip log/print system is not designed for a library use for now. But we could perhaps address those in another PR.

misc suggestions

Not related to this PR but were found during the development process.

• remove release param from rez.createPackage, instead only use the prefix param to determine the path of the installation (that could also be renamed to be more explicit).
• rename rez.getPythonExecutables to getPythonExecutableFromRange ?

example

Here is a contextual use-case (in reference to issue #88 ):

import logging
import shutil
import sys
import tempfile
from pathlib import Path

import rez.package_maker
import rez_pip.main
import rez_pip.pip

LOGGER = logging.getLogger(__name__)


def patch_cyclic_dependency(
    rez_package: rez.package_maker.PackageMaker,
    pip_distribution,
    python_version,
    is_released: bool,
):
    """
    Fix issue on some sphinx dependency that are required by ``sphinx`` but also require
    ``sphinx`` creating a cyclic dependency.

    We simply set a weak reference on all their ``requires`` to fix the issue.
    """
    if not rez_package.name.startswith("sphinx") or rez_package.name == "sphinx":
        return

    LOGGER.info(f"patching {rez_package.name}")
    rez_package.requires = ["~" + require for require in rez_package.requires]


def main():
    tmp_dir = Path(tempfile.mkdtemp(suffix="rez_pip"))
    tmp_dir = tmp_dir / "sphinx"
    tmp_dir.mkdir()

    LOGGER.info("calling rez_pip ...")
    installation = rez_pip.main.run_full_installation(
        pipPackageNames=["sphinx==7.2.6"],
        pythonVersionRange="==3.10.11",
        pipPath=Path(rez_pip.pip.getBundledPip()),
        pipWorkArea=tmp_dir,
        rezPackageCreationCallback=patch_cyclic_dependency,
    )
    LOGGER.info(
        f"installed {len([package for pyv, packages in installation.items() for package in packages])} packages"
    )

    shutil.rmtree(tmp_dir)


if __name__ == "__main__":
    # XXX rez_pip will go crazy if we override its logger
    _root_logger = logging.root
    logging.root = LOGGER
    logging.basicConfig(
        level=logging.INFO,
        format="{levelname: <7} | {asctime} [{name}] {message}",
        style="{",
        stream=sys.stdout,
    )
    logging.root = _root_logger
    main()

todo

• ensure existing tests succeed
• ensure mypy suceed
• add tests for run_full_installation

[codecov]

Codecov Report

Attention: 35 lines in your changes are missing coverage. Please review.

Comparison is base (b593438) 81.91% compared to head (3dc8b08) 81.68%.

Files	Patch %	Lines
src/rez_pip/main.py	37.73%	31 Missing and 2 partials ⚠️
src/rez_pip/rez.py	91.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
- Coverage   81.91%   81.68%   -0.24%     
==========================================
  Files           8        9       +1     
  Lines         719      759      +40     
  Branches      150      153       +3     
==========================================
+ Hits          589      620      +31     
- Misses        115      123       +8     
- Partials       15       16       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JeanChristopheMorinPerso]

Hi @MrLixm thanks a lot for creating this PR and for the great PR description :)

First of all, rez-pip was not designed to be used as an API. I made design choices knowing full well that the API wouldn't be useable as is. That was kind of intentional.

Could you explain in more details what use cases you are facing that rez-pip doesn't cover or what kind of use cases this would enable? I know that we need to fix a couple of bugs, for example the name casing thing or even how PySide is handled. To fix some of these, I've been thinking about implementing a lightweight plugin system that would allow to create hooks, similar to how PyInstaller solves these problems.

Basically, before opening the doors to a public API, I'd like to see if we can look at hooks or other solutions.

Would you like to collaborate on such an effort? I'm definitely searching for collaborators that are active users to make sure that the solution we come up with fits the needs of users.

Let me know what you think.

[MrLixm]

Hello,
Nice to know. I still think for now a public API can be useful, even if minimal. And by minimal I mean instead of doing a subprocess.call() to rez_pip, just having access to the top-most-level function that the CLI is calling to avoid going through subprocess to reproduce the same behavior.

My current context is I am trying to ensure a robust deploy pipeline for pip packages. You can't have any developer calling rez_pip XXX --release affecting the rez central repository without leaving trace of why and how they did it.

So my goal is to have version controlled repository that contain a "install script" for every pip package. So if one would run all those scripts on a fresh new central repository, you would get a mirror of the current one.

This could be in theory just a bunch of powershell/shell scripts that call the CLI, but with the current limitations and issues of rez_pip I have found myself in need of doing a lot of workaround that are made much easier in python. You could argue that you should not decide to change the scope (CLI > library) of a tool because it is currently missing feature and you'd be right, so it is indeed important to have a look if its really pertinent.

Here is some of my current needs:

• my rez python packages have non-standard versioning, in the end rez_pip just needs an official python version for pip, and a python executable. It would be great if I could directly provide those without rez_pip guessing them in-between.
• some of the pip packages I have tried to install have their quirks that need to be worked around:
  • PySide which need shiboken next to it to work
  • Sphinx has circular dependency on some of its extensions, supported by pip but not by rez
  • and we probably found more edge-case in the future
• being able to customize further the rez packages generated with for example new attributes specific to my pipeline
• being able to know exactly what rez_pip did and generate a report out of it. If I call 2 times the same rez_pip command, how many packages are actually created/edited on disk ?

I would be very happy to collaborate with you if I can be of any help.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud