-
Notifications
You must be signed in to change notification settings - Fork 103
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
[WIP] support poetry sources for private pypi repositories #353
[WIP] support poetry sources for private pypi repositories #353
Conversation
✅ Deploy Preview for conda-lock ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Thanks a lot for all this work! I do however have one serious concern, namely that we are favoring Poetry over the alternatives which are more standards-compliant. For example, the Unfortunately I don't yet have any particular system in mind. It could be that we continue leveraging certain vendored components of Poetry, like regarding authentication, while ensuring that the functionality is available to non-Poetry users. Do you have any thoughts? |
I just saw that the CI tests were locked due to your new username. Feel free to ping me if you need CI approval. To elaborate a bit on what I wrote above, I'd be interested to see if we could swap out our vendored Poetry with something more standards-compliant like PDM. They use Given this, my main concern is to ensure that new functionality we add doesn't lock us in further towards Poetry. |
I have to admit, I have very limited understanding of current python build systems - last time I checked, everybody was using setup.py and pip 😬. For me the main motivation here was to make conda-lock and poetry compatible, as for us conda usage is limited to data science folks and supporting both poetry and conda compatible But I agree that poetry is just one system among many and building a more general repository system for conda-lock would have a lot of charm. However, that would require some research and input. Last time I checked, there was no such thing as lock files and package installation order was specified in |
Yes, the state of current Python build systems is quite a mess, although it is improving since the old setuptools days. I'm happy to have Poetry-compatibility so long as it's not at the cost of compatibility with other tools. Research and input is definitely welcome. My own understanding is quite limited (I haven't even used PDM), so I would definitely miss things without these sorts of discussions. |
Okay, having thought about this a bit more: logically, I think the concept of poetry sources and conda channels is completely equivalent, and a lot would become cleaner if instead of defining them separately here, we just use the concept of We could let tools like poetry specify the repositories in pyproject.toml or we just go with the current way that repositories and auth are read out of some config file. At the moment a lot of the logic is a bit tied to poetry, because that's in the end the code that does the solving. But logically the concept of a generalized |
@maresb I refactored the code a bit and now use the channel concept both for conda channels and pypi repositories. I think if there's a need, we could relatively easily generalize adapt the code to support private indices for non-poetry pyproject.yaml. One way would be by defining pypi repositories in a new section |
Thanks @croth1-liveeo! This last commit looks primarily like a substitution I'll try to look more carefully soon. |
If there's interest, I could build support also for the non-poetry-flavored pyproject.toml (albeit not on company time 😬) This PR should move conda-lock a bit closer to fully supporting poetry-style pyproject.toml, while being general enough to not introduce unnecessary poetry internals in the process. I believe the PR should be fully backwards compatible - it just extends the functionality for poetry pyproject.toml. I personally would prefer to extend on this in a separate PR. I will write tests, once you had the chance to go through this PR and have no concerns on a conceptual level 🙂 |
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.
I gave this a shot, but was unable to get it to lock with my private repository. I was deep in the debugger for about 2 hours, but was unable to identify where the solution was. Please lmk if you would like another review.
# read auth from global poetry config | ||
auth_config_file = TOMLFile(Path(user_config_dir("pypoetry")) / "auth.toml") | ||
if auth_config_file.exists(): | ||
config.merge(auth_config_file.read()) |
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.
Can you walk me through what happens here? When would an auth.toml
exsist? Should this instead be looking for a config.toml
?
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.
the auth.toml gets created by poetry config http-basic.<private-source-name> <user> <password>
, see: https://python-poetry.org/docs/repositories/#installing-from-private-package-sources. The code here assumes that for each source you added to your your pyproject.toml that needs login, you provided those by poetry config
prior to attempting to lock.
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.
Oh interesting for my use case i need to be on the companies VPN and i will be able to access to private repo.
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.
I am not an expert, so I might be completely wrong here - but I think it shouldn't matter too much whether you are using VPN or not, as long as during locking and environment creation you are in the same VPN. And in case you still need to authenticate, you can do that with http basic authentication and the auth credentials are provided in the auth.toml file.
|
||
if repositories is not None: | ||
repo_config = {} | ||
for repo in repositories: |
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.
If the user only lists their private repo, but they want to fall back on the default pypi, do they need to list the fallback in their pyproject.toml
?
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.
The pypi fallback is defined by a separate configuration option - they would not have to list pypi as a source, but make sure that allow_pypi_requests
is set to true (which by default it is AFAIK)
conda-lock/conda_lock/pypi_solver.py
Lines 372 to 373 in f09744d
if allow_pypi_requests: | |
repos.append(PyPiRepository()) |
What info could i provide you during my lock that might help troubleshoot why i'm unable to lock? |
@leeleavitt, it's obviously not ideal that a debugger is necessary to understand what the code is doing. Any annotations you could contribute in the form of code comments would be very welcome. |
Please correct me if I'm wrong, but I think the desired functionality is now provided by #529 |
While private sources are currently supported, they do not use the poetry
source
concept in pyproject.toml: While specifyingsource = pypi
will mark a package to be installed with pip instead of conda, it does not allow specifiying which index the package should come from, nor allow configurig the used indices directly inpyproject.toml
.The changes in this PR should make conda-lock aware of poetry sources and also allow providing auth for private sources.
Relies on: #323 for providing auth to private indices during installation.
Still WIP - tests still needed.