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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Poetry and reinstate setup.py #583

Merged
merged 1 commit into from Jun 3, 2019
Merged

Conversation

ob-stripe
Copy link
Contributor

@ob-stripe ob-stripe commented Jun 3, 2019

r? @remi-stripe
cc @stripe/api-libraries

Poetry has a bad bug that prevents building packages from sources (python-poetry/poetry#760), and some environments cannot use prebuilt wheels and must build from source. So we cannot use Poetry 馃槶

I don't want to revert to using pipenv which is also broken in different ways, so I've instead reverted to the pre-2.0 state of not using a dependency manager at all. Sad, but until a Python dependency manager that actually works emerges I think this is the best course of action.

@remi-stripe I know this is typically the sort of PRs where you'd defer to Brandur, but he's on vacation and I think we need a rapid resolution here. I'll add some comments to the PR but lmk if you have any questions.

I've verified that the source distributions for 2.29.1 (last version using pipenv) and this branch are ~identical minus the expected changes (changes to the source code in #579, version number).

Fixes #582.

.gitignore Show resolved Hide resolved
Makefile Show resolved Hide resolved
tests/test_integration.py Outdated Show resolved Hide resolved
Copy link
Contributor

@remi-stripe remi-stripe left a comment

Choose a reason for hiding this comment

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

This looks good to me especially as you've tested. I don't understand the full PR itself though since it's python and build specific so mostly trusting you on that one.

@ob-stripe ob-stripe merged commit 3c65a52 into master Jun 3, 2019
@ob-stripe ob-stripe deleted the ob-manual-setup-py branch June 3, 2019 23:55
@ob-stripe
Copy link
Contributor Author

Released as 2.29.4.

@ob-stripe ob-stripe mentioned this pull request Jun 4, 2019
@koobs
Copy link
Contributor

koobs commented Jun 4, 2019

Thank you for this. This issue blocked updating the FreeBSD Port of the stripe package (and im sure for other OS package maintainers too)

Of particular concern is/was the use of only [~^]X.Y's as a prescription/default for dependency lines, or more precisely, the lack of poetry's ability (it seems?) to do or declare >= style dependency lines.

Pinning like this significantly, if not entirely, precludes downstreams from packaging libraries, and means that in most cases, upstreams also aren't testing the latest and greatest during development due to pinned deps (worst, the == kind), and not picking up dependency conflicts or incompatibilities until way too late down the track, usually via user reports.

This ultimately causes and creates more overhead and uses more time/effort resources for everyone that are better spent fixing bugs or creating new features

So again, thank you!

@ob-stripe
Copy link
Contributor Author

@koobs No worries! And thanks for sharing, this is excellent feedback. I was not really cognizant that there were OS packages for our client library, this is really good to know and something to keep in mind next time we try to make changes to the way the package is built.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot install since integration of poetry
4 participants