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

Poetry support #801

Closed
wants to merge 12 commits into from
Closed

Poetry support #801

wants to merge 12 commits into from

Conversation

AndreGuerra123
Copy link

No description provided.

@AndreGuerra123 AndreGuerra123 marked this pull request as ready for review February 17, 2019 20:05
@@ -0,0 +1,35 @@
#!/usr/bin/env bash

# Detect Python-version with Pipenv.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should Pipenv be Poetry here?

@CaseyFaist CaseyFaist mentioned this pull request Feb 20, 2019
@CaseyFaist
Copy link
Contributor

CaseyFaist commented Feb 20, 2019

Hi there @AndreGuerra123 !

Thanks for the PR! I may be able to iterate on this with you in the next few weeks - poetry support is on the roadmap for the buildpack 🎉

In the meantime, can you add some details to your description about what Poetry is for those who don't know, and a bit about what your implementation strategy was?

@CaseyFaist
Copy link
Contributor

I'm also seeing this coming from unknown repository with a lot of fix commits - if you can work to clean up the git history and origin of this branch, that would help make this branch much more easily mergeable 👍

@AndreGuerra123
Copy link
Author

Hi, I really want to progress on this and I'm trying to implement some ideas but I kinda need your help here. First of all, I want to ask you how do you proceed with the development and testing. Currently, I have a virtual machine on Vultr with docker and every time I need to test my code I run make test, but it usually takes too long. Worst than that, I cannot see the error message when the build fails. Is it any other way to run the test more efficiently and retrieving more information? How do you do it?

About the overall buildpack:

  • I changed the detect script to detect a python project with poetry.lock and Pipfile.lock files aswell. I think that If the user wants to submit only the exact dependencies that should be possible and would result in more specificity on the dependencies installed.
  • Along with the previous point, I developed methods to retrieve the pytho version from bothe pipenv and poetry (priority to lock version over config version, priority of full version over major version) which passes the tests but could be further improved.
  • During Pipenv installation and installation of the designated packages through pipenv, there are some points (steps) that to me seem unnecessary (maybe I am wrong) which I will try to clarify here:
  1. The lock file checking to inspect alterations and skip pipenv installation. Shouldn't it be the responsibility of the package manager to check if the correct versions of the packages are installed or need update?
  2. From my understanding, couldn't we run the pip freeze command inside of the virtualenv (with the packages previously installed with pipenv) and remove the steps of uninstalling pip artifacts?
  3. Why export requirements.txt and run pip install later on at all, if pipenv can install packages system-wide?

About the poetry implementation, similar implementation and questions are pertinent:

  • Similar implementation to pipenv with python version being extracted from configuration files.
  • Installation through pip of poetry with poetry version being extracted from configuration files (which seems to be fixed on Pipenv).
  • Poetry as far as I know does not have a system wide install. Always installs in a virtualenv which the we could do something similar to poetry run pip freeze >requirements.txt and proceed with pip install.

Hope I can get some clarification and my ideas are align with your goals,
Andre

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.

3 participants