conda / conda-lock

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

Configure private pip repositories in the environment file #529

Closed jacksmith15 closed 11 months ago

jacksmith15 commented 11 months ago

Description

[!NOTE] This is a rebase of #481 with a tidier commit stack.

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.

netlify[bot] commented 11 months ago

Deploy Preview for conda-lock ready!

Name Link
Latest commit 0cbbd790470e3e31906aae4b05d7f506b72fcfd9
Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/652e7d15bca19b0009909933
Deploy Preview https://deploy-preview-529--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

Sorry for not getting to this sooner. The data structure for what I review is more of a stack than a queue, and the previous PR had unfortunate timing with my availability. Thanks so much for your patience and the rebase!!!

jacksmith15 commented 11 months ago

@maresb I'm not sure why the single test is failing on windows, It doesn't appear relevant to the changes I've made. Possibly related to this being a fork PR?

maresb commented 11 months ago

I think the Windows runner is getting overloaded. It's not specific to this PR. Don't worry about it, and I can force the merge if needed.

maresb commented 11 months ago

I added a test for the pip repo aggregation in https://github.com/jacksmith15/conda-lock/pull/1

maresb commented 11 months ago

I don't think we know for sure that the repository order is actually respected, do we?

How hard would it be to add a test using your nifty framework to check that if we have two repositories a and b, then the first listed is the first one to be queried?

jacksmith15 commented 11 months ago

I don't think we know for sure that the repository order is actually respected, do we?

I believe this is poetry's behaviour. The find_packages method of Pool checks repositories in the order they were provided to the pool:

https://github.com/conda/conda-lock/blob/main/conda_lock/_vendor/poetry/repositories/pool.py#L169-L170

This method is called by the solver here.

How hard would it be to add a test using your nifty framework to check that if we have two repositories a and b, then the first listed is the first one to be queried?

It would be a chunk of code, and we'd mostly be testing poetry, but happy to add that if you want it confirmed?

jacksmith15 commented 11 months ago

Actually that's not entirely true! It looks like Poetry sorts the versions from all available repositories:

https://github.com/conda/conda-lock/blob/1544dc50b3359b3d04b2e75689b3c717b8bcd4e4/conda_lock/_vendor/poetry/puzzle/provider.py#L141C1-L148C1

So it will pick the latest version from among all of them.

If the latest matching version is available in multiple repositories, it will follow the priority order as before, but if a repository further down the priority order has a later version, it will take that one.

I suppose that means we don't need the unifying logic here at all

EDIT: Actually the unifying logic does still make sense in the case where the same versions are available in multiple repositories, as there is still a priority order to respect in this case.

maresb commented 11 months ago

I'm thinking that ultimately it probably doesn't make sense to merge channels. It would seem more logical to me if each package were solved within the context of only the sources defined in the same specification file. But I don't know that Conda and Poetry are capable of per-package sources.

But rewriting everything from scratch is a bit out-of-scope :joy:

jacksmith15 commented 11 months ago

I'm thinking that ultimately it probably doesn't make sense to merge channels. It would seem more logical to me if each package were solved within the context of only the sources defined in the same specification file. But I don't know that Conda and Poetry are capable of per-package sources.

But rewriting everything from scratch is a bit out-of-scope 😂

Yes unfortunately the poetry solver doesn't support this last time I checked. Its a problem that I haven't seen considered in dependency resolution algorithms.