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

Replace "poetry" dependency with "poetry-core" #2

Open
enpaul opened this issue Sep 29, 2020 · 9 comments
Open

Replace "poetry" dependency with "poetry-core" #2

enpaul opened this issue Sep 29, 2020 · 9 comments
Labels
feature New feature or request roadmap-stable Required for stable status

Comments

@enpaul
Copy link
Owner

enpaul commented Sep 29, 2020

poetry-core is a project to produce a lighter weight version of core poetry functionality specifically to better support PEP-517 build systems.

However (based on reading the published goals as of the 1.0 RC1 release) I believe it will also contain all the functionality necessary for this plugin to function.

Using the poetry-core package instead of poetry would give this project two main advantages:

  1. By design the poetry-core dependencies are considerably lighter than those of poetry. This was done to speed up the creation of PEP-517 build environments, but here it would reduce the size of this project's dependency chain. Since this is supposed to be a minimalist, lightweight plugin that just bridges the functionality of Tox and Poetry it would be nice if it did not need to install a huge number of dependencies to function.
  2. The poetry-core package will provide the poetry.core module namespace and replace a decent amount of functionality currently in the main poetry module. This means, at minimum, this plugin will need to be updated to be compatible with Poetry 1.1.0+ (which will use poetry-core). However due to the design goals of poetry-core it will, by necessity, have a more stable API than some of the more esoteric elements of poetry that this plugin currently imports. Using poetry.core instead of poetry will reduce the likelihood of future updates to Poetry breaking this plugin.

I need to confirm what- if any- functionality of poetry that this plugin needs is not included in poetry-core and determine how to work around those limitations. At minimum I suspect some of the types that are currently imported will not be available, but I have not confirmed this yet.

@enpaul enpaul added feature New feature or request roadmap-beta labels Sep 29, 2020
@enpaul
Copy link
Owner Author

enpaul commented Oct 11, 2020

This is blocked for the moment, on several components:

@johnthagen
Copy link

This would be very nice. The issue we ran into is that if you put tox-poetry-installer in the dev-dependencies and install into a production environment (such as a Docker container) using poetry install --no-dev, because tox-poetry-installer depends on poetry, this would uninstall poetry itself since in a Docker container, there is typically no need for a separate virtual environment for Poetry itself.

We needed to move back to the [tox] requires method for now.

@enpaul
Copy link
Owner Author

enpaul commented Oct 23, 2020

@johnthagen As a workaround for the issue I believe that installing poetry using the get-poetry script will prevent it from uninstalling itself from the project environment. I haven't tested this exact scenario, but I had a similar issue in a docker build where my poetry installation existed alongside my project installation, and switching to installing poetry via the script fixed it for me.

@johnthagen
Copy link

@enpaul Thanks, I suspected something like that would help as well.

@enpaul
Copy link
Owner Author

enpaul commented Oct 24, 2020

Taking another look at this one. The main blocker here is definitely the lockfile support in poetry-core. Parsing and handling lockfile conditions is something I would really like to avoid having to reimplement for this plugin.

That said the other blocker I highlighted above (pip installation support) is something I think we can work around. Poetry is MIT licensed so I think the PipInstaller class (and any other necessary plumbing for it) should be fairly simple to vendorize without too much trouble. Before doing this though I will want to review what is in the PipInstaller class to make sure it's viable.

Once both of those elements are unblocked I can drop the poetry dependency.

@johnthagen
Copy link

Parsing and handling lockfile conditions is something I would really like to avoid having to reimplement for this plugin.

Yes, I totally agree with this. I'd never want to ask a package like this to reimplement something that should otherwise be shared with poetry. We have working work-arounds and understand the poetry ecosystem still has a little maturing to do.

Thanks!

enpaul added a commit that referenced this issue Dec 4, 2020
When running 'poetry remove tox-poetry-installer' in the same env as
poetry is installed to, poetry will uninstall itself. This is, obviously,
not ideal.

This change makes poetry an optional dependency so that the plugin can be
installed (and uninstalled) alongside poetry in the same env without
breaking the poetry installation. The intention is that the plugin can be
installed with the 'poetry' extra when being installed to an isolated environment
where poetry is not otherwise available.

This is a mitigation of Issue #2 as an alternative to vendorization of the enitre
poetry project 😬
@enpaul enpaul added roadmap-stable Required for stable status and removed roadmap-beta labels Dec 16, 2020
@johnthagen
Copy link

@enpaul FYI, on the main README this is checked as complete, even though it looks like actually it was just deferred until the stable release.

@enpaul
Copy link
Owner Author

enpaul commented Jan 20, 2021

Ah, good catch @johnthagen . I'll update the readme

@enpaul
Copy link
Owner Author

enpaul commented Feb 24, 2023

Update on this issue: this is currently blocking both the 1.0 release of this plugin and the upgrade to Tox 4 compatibility (as noted elsewhere, there are compatibility issues between the versions of importlib-metadata required by Tox 4 and Poetry 1.x). Once this issue is resolved I'll plan to build and release version 1.0 that includes Tox 4 support and drops the Poetry dependency, which solves another bunch of issues (including #82 and #79)

The singular blocker on this right now is lockfile processing in poetry-core, being tracked in python-poetry/poetry-core#83. Once that's merged and/or the functionality is otherwise added this issue becomes unblocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request roadmap-stable Required for stable status
Projects
None yet
Development

No branches or pull requests

2 participants