Skip to content
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

Closed

Conversation

croth1-liveeo
Copy link

@croth1-liveeo croth1-liveeo commented Feb 16, 2023

While private sources are currently supported, they do not use the poetry source concept in pyproject.toml: While specifying source = 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 in pyproject.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.

@croth1-liveeo croth1-liveeo requested a review from a team as a code owner February 16, 2023 11:05
@netlify
Copy link

netlify bot commented Feb 16, 2023

Deploy Preview for conda-lock ready!

Name Link
🔨 Latest commit f09744d
🔍 Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/63f0da59b220980008f6f836
😎 Deploy Preview https://deploy-preview-353--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 settings.

@croth1-liveeo croth1-liveeo marked this pull request as draft February 16, 2023 11:05
@maresb
Copy link
Contributor

maresb commented Feb 16, 2023

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 source = "pypi" key-value pair is not standards-compliant, so unless I'm mistaken it can only really be used for projects using Poetry as a build system. I'd really like to see this somehow eliminated.

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?

@maresb
Copy link
Contributor

maresb commented Feb 16, 2023

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 project.dependencies, which would unfortunately leave basic auth credentials in pyproject.toml. But to get around this, they also support environment variable expansion.

Given this, my main concern is to ensure that new functionality we add doesn't lock us in further towards Poetry.

@croth1-liveeo
Copy link
Author

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 pyproject.toml seems very advantageous.

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 pip.conf. 😬

@maresb
Copy link
Contributor

maresb commented Feb 16, 2023

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.

@croth1-liveeo
Copy link
Author

croth1-liveeo commented Feb 16, 2023

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 channel, which is a conda_channel in case of a conda dependency and the name of some kind of python pypi repository in case of a pip repository.

https://github.com/conda/conda-lock/pull/353/files#diff-c6519186a02b9314f4ed76075c3b00068f2a440442c6ee64c0690f44c82ce3e0R61-R62

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 channel that provides the package should be the same.

@croth1-liveeo
Copy link
Author

@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 tool.conda-lock.pypi_repositories (similarly to poetry sources) and then passing the auth via command line e.g. If we wanted to also support defining a specific source repository for packages, we would need another tool.conda-lock section, that would provide this mapping.

@maresb
Copy link
Contributor

maresb commented Feb 18, 2023

Thanks @croth1-liveeo!

This last commit looks primarily like a substitution poetrypypi, which actually does make me feel a bit better. Am I understanding correctly that the channel concept would be future work?

I'll try to look more carefully soon.

@croth1-liveeo
Copy link
Author

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 🙂

Copy link

@leeleavitt leeleavitt left a 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())

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?

Copy link
Contributor

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.

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.

Copy link
Contributor

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:

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?

Copy link
Contributor

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)

if allow_pypi_requests:
repos.append(PyPiRepository())

@leeleavitt
Copy link

What info could i provide you during my lock that might help troubleshoot why i'm unable to lock?

@maresb
Copy link
Contributor

maresb commented Mar 23, 2023

I was deep in the debugger for about 2 hours

@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.

@maresb
Copy link
Contributor

maresb commented Oct 17, 2023

Please correct me if I'm wrong, but I think the desired functionality is now provided by #529

@maresb maresb closed this Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants