-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Configure private pip repositories in the environment file #529
Configure private pip repositories in the environment file #529
Conversation
✅ Deploy Preview for conda-lock ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really amazing. This looks like it was a huge amount of work! Found a tiny bit of time to look at this. I have a few questions about suffix_union...
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!!! |
Co-authored-by: Ben Mares <services-git-throwaway1@tensorial.com>
@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? |
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. |
I added a test for the pip repo aggregation in jacksmith15#1 |
…file Add test for PipRepository aggregation
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 |
I believe this is poetry's behaviour. The This method is called by the solver here.
It would be a chunk of code, and we'd mostly be testing poetry, but happy to add that if you want it confirmed? |
Actually that's not entirely true! It looks like Poetry sorts the versions from all available repositories: 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.
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. |
Co-authored-by: Ben Mares <services-git-throwaway1@tensorial.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jacksmith15!!!
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. |
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.
This aims to solve #460.