conda / conda-lock

Lightweight lockfile for conda environments
https://conda.github.io/conda-lock/
Other
458 stars 102 forks source link

Configure private pip repositories in the environment file #481

Closed jacksmith15 closed 8 months ago

jacksmith15 commented 11 months ago

Description

This PR adds support for specifying private pip repositories in the environment.yml file, similar to how channels are specified.

Similarly to channels, environment variables may be specified, and these will remain as references in the lockfile.

channels:
  - conda-forge
pip-repositories:
  - http://$PIP_USER:$PIP_PASSWORD@private-pypi.org/api/pypi/simple
dependencies:
  - python=3.11
  - requests=2.26
  - pip:
    - fake-private-package==1.0.0

This aims to solve https://github.com/conda/conda-lock/issues/460, and is an alternative approach to the one proposed by https://github.com/conda/conda-lock/pull/471

netlify[bot] commented 11 months ago

Deploy Preview for conda-lock ready!

Name Link
Latest commit 49e624f6a405ffcdeda4a77700ab80c3e406506d
Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/6501c88db222770008e27034
Deploy Preview https://deploy-preview-481--conda-lock.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

maresb commented 11 months ago

Hey, I took a look, and I like the direction this is heading, thanks so much for this!

I'm impressed by your PyPI mock. :)

I see that the first commit is primarily a combination of moving code from channel.py to package_source.py, pushing through the pip_repositories argument, and adding a tiny bit of code to make the pip_repositories argument work. But given the large subtractions and additions to channel.py and package_source.py, it took quite some effort to manually diff everything before I managed to deduce this. It would help me a lot with review if you put any subsequent cut-paste operations into their own commits so that I can distinguish between moved code and altered code.

Finally, feel free to be bold about refactoring and simplifying existing code if it helps. You're touching a lot of code that's probably long overdue for an overhaul. For example, I'd prefer to use libraries when it makes things easier.

jacksmith15 commented 11 months ago

I note an issue with the current approach, which is unfortunately not detected by the test.

The poetry solver makes requests to the repository using requests, which appears to automatically swap URL-based Basic Auth with header-based. This is relatively sensible, however it does mean that this debug warning will almost always fire.

More annoyingly, the solver uses the URL from the response not the one used to make the request. This means that if the solver makes a request to https://username:password@private-pypi.org/simple/package, the resulting URL for the package returned by the solver will be https://private-pypi.org/simple/package, which obviously doesn't include the secrets, but also doesn't include the environment variables.

I can only think of one good solution here, which is to inspect each resolved package URL and attempt to match them to the configured pip-repositories, and then insert the basic auth back into the URL (as environment variables). The link objects returned by the solver can be used to match on repositories, because they include they include a reference to the Page and headers of the request.

I'll investigate this route.

:memo: This might be more straightforward to implement (i.e. fewer edge cases, more predictable for users) if the configuration format were more restrictive. E.g.

pip-repositories:
    - url: https://private-pypi.org/pypi/simple
      basic-auth:
        username: $USERNAME
        password: $PASSWORD
maresb commented 11 months ago

One of my top priorities is to upgrade the vendored version of Poetry. I'm not sure whether or not this would help, but anyways I want to depend on Poetry as little as possible. But if it's tied into their solver I suppose we have no choice but to play along for now, since we can't modularize the solver overnight.

mariusvniekerk commented 11 months ago

We should also support parsing credentials dynamically out of netrc files (this works quite well for a lot of conda package registries)

jacksmith15 commented 10 months ago

@maresb @mariusvniekerk

I (finally) found enough time to get this in a working state, ready-for-review. Let me know what you think.

maresb commented 10 months ago

@jacksmith15 I'm doing a lot of traveling at the moment; this is in my review queue, but I'm still looking for a chunk of time when I can go through this carefully. Feel free to pester me, but I can't promise any timescale yet. Perhaps @mariusvniekerk could help if available.

jacksmith15 commented 8 months ago

@maresb I've re-opened this PR in https://github.com/conda/conda-lock/pull/529 with up-to-date changes from main and a cleaner git history. Keen to get your thoughts.