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

Bundle command implementation #2682

Closed
wants to merge 2 commits into from
Closed

Bundle command implementation #2682

wants to merge 2 commits into from

Conversation

sdispater
Copy link
Member

@sdispater sdispater commented Jul 17, 2020

This PR adds a new bundle venv command as discussed initially in #537 and formalized in #1992.

Basically, this command creates a virtual environment in the given directory and install the dependencies of the project and the project itself inside it.

Pull Request Check List

Resolves: #537
Resolves: #1992

  • Added tests for changed code.
  • Updated documentation for changed code.

@sdispater sdispater added area/cli Related to the command line area/venv Related to virtualenv management labels Jul 17, 2020
@sdispater sdispater added this to the 1.1 milestone Jul 17, 2020
@sdispater sdispater marked this pull request as ready for review July 17, 2020 14:20
@sdispater sdispater requested a review from a team July 17, 2020 14:20
@sdispater sdispater mentioned this pull request Jul 17, 2020
14 tasks
@abn
Copy link
Member

abn commented Jul 18, 2020

I feel like we are re-implementing a fair amount of logic here wrt building, env management, and installation. I am not a 100% sure if it is out of necessity (haven't looked at the code changes in detail yet). With the creation of the virtualenv, we should be careful in that the environment created should not use symlinks etc for platlib (not sure how this is handled out of the box). Additionally, can't we skip installation of setuptools/pip/wheel etc during environment creation? Since we can rely on the development environment's pip to handle installation to specified site right?

Overall I am a bit weary of this functionality, while useful, introduces a fair amount of fragility.

@sdispater
Copy link
Member Author

I feel like we are re-implementing a fair amount of logic here wrt building, env management, and installation.

I don't understand. This command does not re-implement anything but uses existing modules (env management, building and installation) in a specific way, i.e. to bundle the project and its dependencies in a "portable" way.

The purpose of the bundle sub commands is to do exactly that. The idea is to make it extensible so that it can be used to bundle projects in various, ready-to-deploy formats.

@Imaclean74
Copy link
Contributor

I haven't looked thru the implementation in detail - but is the goal to achieve something like what shiv is doing ? I've been using poetry build + shiv to produce a deployable artifact. Great if this can all be done thru poetry. I guess the key difference is that Shiv is also packing things into an executable .zip ( .pyz ), but otherwise the packaging of dependencies looks pretty similar.

docs/docs/cli.md Outdated
If the virtual environment already exists, two things can happen:

- **The python version of the virtual environment is the same as the main one**: the dependencies will be synced (updated or removed).
- **The python version of the virtual environment is different**: the virtual environment will be recreated from scratched.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scratch ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked thru the implementation in detail - but is the goal to achieve something like what shiv is doing ? I've been using poetry build + shiv to produce a deployable artifact. Great if this can all be done thru poetry. I guess the key difference is that Shiv is also packing things into an executable .zip ( .pyz ), but otherwise the packaging of dependencies looks pretty similar.

@Imaclean74 any chance you can provide more details on this?

@bratao
Copy link

bratao commented Sep 8, 2020

+1, just to contribute to the conversation. This feature is a must for FaaS scenarios such as AWS lambda, where we need to deploy a fully contained app, with all the dependencies @abn @sdispater

docs/docs/cli.md Outdated
If the virtual environment already exists, two things can happen:

- **The python version of the virtual environment is the same as the main one**: the dependencies will be synced (updated or removed).
- **The python version of the virtual environment is different**: the virtual environment will be recreated from scratched.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scratch

Suggested change
- **The python version of the virtual environment is different**: the virtual environment will be recreated from scratched.
- **The python version of the virtual environment is different**: the virtual environment will be recreated from scratch.

@abn abn removed this from the 1.1 milestone Sep 23, 2020
@abn abn marked this pull request as draft September 27, 2020 10:56
@abn
Copy link
Member

abn commented Sep 27, 2020

@sdispater with all the things going on around venv/zipapp/wheelhouse etc; I think I need to revisit my stance on this. I think I am amenable to this top-level command being introduced to poetry. Moved this to draft for now. Once this is doen I want to do a PEP 441 bundler as well.

Moving this to draft for now in order verify things are still up-to-date. Will review once rebased and marked as ready again.

@Imaclean74
Copy link
Contributor

@sdispater with all the things going on around venv/zipapp/wheelhouse etc; I think I need to revisit my stance on this. I think I am amenable to this top-level command being introduced to poetry. Moved this to draft for now. Once this is doen I want to do a PEP 441 bundler as well.

Moving this to draft for now in order verify things are still up-to-date. Will review once rebased and marked as ready again.

+1 on the zipapp bundler. Done right - it would remove the need to use a separate tool like shiv or pex

@garthk
Copy link

garthk commented Dec 18, 2020

It's possible to clone the workings of the poetry/bundle/venv_bundler.py in this pull request, eg. to end up with this bundler.py you can copy into any repository with a tool.poetry.dev-dependencies entry for poetry itself. Yes, a proper integrated bundle command would be better.

@sdispater sdispater changed the base branch from develop to master February 12, 2021 17:23
@sdispater
Copy link
Member Author

Closing in favor of an official plugin (https://github.com/python-poetry/poetry-bundle-plugin) which will be made extensible so that users can add their own bundlers to the command.

@sdispater sdispater closed this Jul 9, 2021
@sdispater sdispater deleted the bundle-command branch July 9, 2021 13:48
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cli Related to the command line area/venv Related to virtualenv management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bundle command Feature Request: poetry deploy
7 participants