open-contracting-archive / kingfisher-vagrant

Abandoned as not kept up-to-date with Kingfisher components
BSD 3-Clause "New" or "Revised" License
5 stars 5 forks source link

Choose a python dependency management methodology/tool for apps (incl Kingfisher + others) #329

Closed robredpath closed 4 years ago

robredpath commented 4 years ago

Edit by @jpmckinney: Can revert https://github.com/open-contracting/standard-maintenance-scripts/pull/141 once this is closed.

odscjames commented 4 years ago

This ticket is to recognise that the current system of requirements.in/.txt files could do with a review. The goal is to have a good system of locking requirements in apps. There are fortunately in modern Python several options for this - Pipenv is one, a lot of people say Poetry is better. We don't have strong opinions at this point. We want to review a few of them and then have a discussion about this.

(We've put this ticket in kingfisher but it applies to any apps; ocdskitweb/toucan for instance)

jpmckinney commented 4 years ago

pip-tools was another. I'm not crazy about Poetry being essentially/almost its own separate dependency management system; I'd prefer to stick with simpler pip-based tools.

odscjames commented 4 years ago

This is https://pypi.org/project/pip-tools/ ? Looks interesting.

jpmckinney commented 4 years ago

That's the one. My main complaint about Poetry is that (from my very quick look) it needs its own config file in each repo, and developers would then need to install Poetry in order to work on a repo (i.e. even just to install dependencies), whereas pip comes with Python.

robredpath commented 4 years ago

@odscjames is looking to what pip-tools looks like in practice this afternoon

odscjames commented 4 years ago

I've had a look, and I think for apps (not libs) pip-compile should work well. I'll check my work with others here and write a full report soon.

jpmckinney commented 4 years ago

Great, I think this is only for apps anyway, as libs don't have req files.

robredpath commented 4 years ago

@odscjames will share his work shortly

odscjames commented 4 years ago

Great, I think this is only for apps anyway, as libs don't have req files.

Yes

My main complaint about Poetry is that (from my very quick look) it needs its own config file in each repo

You mean pyproject.toml ? I was actually at a local Python pub event this week so I asked about this topic as a whole, and Poetry. Apparently Python as a whole is officially moving in this direction and this file in the future will power a lot of tools. Hearing that, I'd like to have a little play with Poetry and see how it works.

pip-tools was another.

So I did have a play with pip-tools, and it's nice. For apps it works well with no setup.py and 4 files in the root of the repository (requirements_dev.in, requirements_dev.txt, requirements.in, requirements.txt). A minor issue about how you reference one file from another I want to play with a bit more, but it all works well. Also, the lock files you get from it are really nice; for indirect dependencies it will put in a comment telling you which direct dependency needed it.

Next; I could have a look into poetry and this thing about pyproject.toml, and/or setup up pip-tools on a branch in a real project to let people see how that works.

jpmckinney commented 4 years ago

I prefer to move ahead with pip-tools, as it looks fairly low effort to implement and it is the more conservative option in terms of guessing the future.

My past experience with community opinions of where Python is moving is that either it doesn’t happen or it takes a decade (as with Python 3).

In this case, I’d rather not be an early adopter.

On Friday, October 25, 2019, James (ODSC) notifications@github.com wrote:

Great, I think this is only for apps anyway, as libs don't have req files.

Yes

My main complaint about Poetry is that (from my very quick look) it needs its own config file in each repo

You mean pyproject.toml ? I was actually at a local Python pub event this week so I asked about this topic as a whole, and Poetry. Apparently Python as a whole is officially moving in this direction and this file in the future will power a lot of tools. Hearing that, I'd like to have a little play with Poetry and see how it works.

pip-tools was another.

So I did have a play with pip-tools, and it's nice. For apps it works well with no setup.py and 4 files in the root of the repository ( requirements_dev.in, requirements_dev.txt, requirements.in, requirements.txt). A minor issue about how you reference one file from another I want to play with a bit more, but it all works well. Also, the lock files you get from it are really nice; for indirect dependencies it will put in a comment telling you which direct dependency needed it.

Next; I could have a look into poetry and this thing about pyproject.toml, and/or setup up pip-tools on a branch in a real project to let people see how that works.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/open-contracting/kingfisher/issues/329?email_source=notifications&email_token=AAAGOX5CYB4AUP7E4MBWBHLQQLSEZA5CNFSM4I6PDUVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECIHAOA#issuecomment-546336824, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGOX6NWLEJU7AXKBGDU6TQQLSEZANCNFSM4I6PDUVA .

--

James McKinney

Head of Data Products and Services

+1-514-247-0223 | @mckinneyjames | skype: mckinney.james | timezone: EST

Reform is hard but you are not alone! Join our growing global community https://groups.google.com/a/open-contracting.org/forum/#!forum/standard-discuss today to connect to likeminded public contracting reformers from 50+ countries. www.open-contracting.org | follow us @opencontracting

[image: OC_logo_sig.png]

odscjames commented 4 years ago

I tried this out

The files are basically the same as before. requirements_dev.in should have the rows:

    -r requirements.txt
    pip-tools

(In one case while playing I did see a problem with conflicting versions of six, and to work round it I changed it to -r requirements.in - but that's not ideal. I'd like to track that down.)

To update requirements, simply run

    pip-compile --upgrade 
    pip-compile --upgrade  requirements_dev.in

You may have to "pip install pip-tools", but only when converting repositories - after that, by virtue of the fact it's in requirements_dev.in, any dev that starts a new virtual environment and runs "pip install -r requirements_dev.txt" will already have it.

Updating one requirement only with --upgrade-package seems ok.

    pip-compile --upgrade-package flask
    pip-compile --upgrade-package Django requirements_dev.in

"pip-sync requirements_dev.txt " and "pip-sync requirements.txt " work as promised - cheekily, the latter will leave pip-tools installed for you even if pip-tools is not in requirements.txt (which it isn't). I can see why tho; it means you can easily switch between the 2 modes without having any "pip-sync is not installed" errors.

I had a look at hashes but found some things more complex with them; using "-e" git URL requirements, which is sometimes very handy to be able to do while developing things. You also get a much messier lock file. Are we worried about the problem the hashes functionality is designed to solve? (Making sure packages don't change once released, either accidentally or maliciously)

In terms of testing this fully, it's a bit difficult; the real test of this will come over time when new versions of packages are installed and we have to upgrade to them! You can test that a bit by hand editing the *.txt lock files to have older versions of libraries and then running upgrade comands, but it's not the same. For that reason, if we decide to go ahead with this I'd suggest doing it on one app only for a bit. I'm happy for that to be one of the bits of Kingfisher. I would choose process, as that has more tests.

There is one thing I'd still like to nail down, and it's maybe a bit unfair as this is outside pip-tools scope, but I'd like to think about ways to make sure when people run package upgrade tools like this they do it on the same version of Python that we use on live. But I don't think there is anything clever we can do there. So maybe leave that.

Anyway, I'm going to be very busy next week but we'll talk soon about this,

jpmckinney commented 4 years ago

I don't think we need the --generate-hashes option.

I'm fine with using this approach more broadly, as the impacts are mainly (1) that the requirements contents are re-ordered and (2) that developers use a different command to update them. Toucan and Extension Explorer are the best-tested apps, though they don't use requirements-dev files. Otherwise, cove-oc4ids has the next best coverage. Kingfisher apps have the least coverage. https://github.com/open-contracting/standard-maintenance-scripts/blob/master/badges.md

For reference re: Poetry, I found the related PEPs, which are still provisional (i.e. who knows what will happen):

odscjames commented 4 years ago

Make sure all repositories follow a consistent scheme for dependencies, have updated requirements, and it's appropriately documented

robredpath commented 4 years ago

I think this is all resolved, at least sufficiently well and for now. Closing.