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

Document how to upgrade pip requirements files #1155

Merged
merged 5 commits into from Nov 12, 2013

Conversation

vitorbaptista
Copy link
Contributor

@ghost ghost assigned vitorbaptista Aug 5, 2013
@seanh
Copy link
Contributor

seanh commented Sep 9, 2013

@vitorbaptista Do you know if the above wiki page is still valid? If yes, can we get it moved into the Sphinx docs?

@vitorbaptista
Copy link
Contributor

@seanh It's not :/ Just tested it and it failed first because CKAN wasn't installed on my virtualenv (added a python setup.py develop step), but some tests fail because we haven't installed dev-requirements.txt.

If we do install the dev-requirements.txt, everything will be fine, but then pip freeze > requirements.txt will add those extra requirements. I can only think of 2 solutions:

  1. Live with it. We forget about dev-requirements.txt and only use requirements.txt;
  2. Add instructions to manually separate the dependencies in both files, using what's already in dev-requirements as a guide;

I don't think we should do 1, as there're dependencies that really don't belong in production servers. As for 2, we might (probably?) have problems with sub-dependencies. It's easy to keep pep8 on dev-requirements when it's updated, but how can I know that a new package was installed/removed because pep8's dependencies changed?

As we have no tool support, I think the best we can do is to go with 2, and hope that jazzband/pip-tools#10 gets solved soon enough, as they're fixing this exact problem (or solve it ourselves).

@domoritz Thoughts?

@domoritz
Copy link
Contributor Author

I guess, we should remove the "run the tests" step and say "test CKAN extensively" :-).

@vitorbaptista
Copy link
Contributor

@seanh @domoritz

I had an idea. You could install requirements.in, upgrading the packages. Then do pip freeze > requirements.txt, and then install dev-requirements.txt. After that, run the tests, check if everything went well. If not, things start getting (more) complicated. You could either fix whatever went wrong, and make CKAN compatible with the new version. Or you'll have to pinpoint what dependency broke our tests (hopefully it's only one), find the latest working version, and add that to requirements.in. For example, if apachemiddleware 0.2 broke our tests, you would change requirements.in to:

apachemiddleware>=0.1.1,<0.2

Then recreate a new virtualenv, and do everything again with this changed requirements.in. Hopefully, this time everything will go well and we'll have new version.

Complex, but it was the best I could think of. Do you know a better option?

@vitorbaptista
Copy link
Contributor

@seanh @domoritz ping

@domoritz
Copy link
Contributor Author

This sounds exactly like what we would want to do, imho. We talked about this earlier and said we should to it early in a new release to make sure that everything works with the new requirements.

It is cumbersome but probably the best we can do.

@vitorbaptista
Copy link
Contributor

Cool, I'll go ahead and write it down in the docs, then. Hopefully it won't take long until pip-tools stabilize and we're able to use it.

@vitorbaptista
Copy link
Contributor

@domoritz @seanh Done. I've also went ahead and upgraded the dependencies following these steps. I'll file a pull request as soon as the tests pass.

Not sure if Contributing is the best place for these docs to live, but I couldn't think of anything better.

Whoever merges, please, delete this page https://github.com/okfn/ckan/wiki/How-to-Upgrade-Requirements (or create a link there to their new place)

@vitorbaptista vitorbaptista mentioned this pull request Oct 21, 2013
@vitorbaptista
Copy link
Contributor

@amercader @domoritz @seanh Maybe it makes more sense to add it to the testing page?

@ghost ghost assigned seanh Oct 22, 2013
@seanh
Copy link
Contributor

seanh commented Oct 28, 2013

Hmm, I think this is fine in CONTRIBUTING at least for now, I can't think of a better place for it.

@vitorbaptista
Copy link
Contributor

Cool, so it's ready to be merged 👍

@seanh
Copy link
Contributor

seanh commented Oct 31, 2013

Contributing does seem wrong though. Leave this assigned to me, and I may move it before merging. I'm off keyboard for at least another week though :(

@vitorbaptista
Copy link
Contributor

OK. I'm starting to feel some aching on my wrists as well. Already moved to
workman layout to avoid anything more serious :/

@seanh
Copy link
Contributor

seanh commented Nov 11, 2013

Hey @vitorbaptista , this is excellent documentation. I've moved it into its own file. The way CONTRIBUTING.rst is organized right now is stupid, because I wanted it to render properly both on docs.ckan.org and on GitHub, but I may drop that requirement and reorganize it in a different pr so we can use proper Sphinx cross-references and tables of contents in there.

One minor thing:

If not, try to fix the problem. If it’s too complicated, pinpoint which dependency’s version broke our tests, find an
older version that still works, and add it to requirements.in. Go back to step 1.

  1. Should that be add it to requirements.in with a pinned version number?
  2. Pinpoint which dependency’s version broke our tests - I suppose there's no recipe for pinpointing this, you just have to debug it?

Also maybe add a sentence under the Steps to upgrade title saying these steps will upgrade every dependency to the latest version that works with CKAN.

It may be worth having a procedure for when you just want to upgrade one dependency too, but that can wait.

@ghost ghost assigned vitorbaptista Nov 11, 2013
@vitorbaptista
Copy link
Contributor

@seanh Thanks!

I've added an explanation that you're supposed to limit the dependency's version in requirements.in, and not pin it. About 2, you're right. There's no recipe for it, unfortunately. It will be VERY hard, depending on the number of dependencies being updated at the same time. For this reason, we have to take care and keep updating regularly (#1292).

Without tool support, there's no way to upgrade a single dependency, while guaranteeing that we remove any indirect dependency that's not needed anymore :/

@seanh seanh merged commit 4c05807 into master Nov 12, 2013
@seanh seanh deleted the 1155-document-upgrading-dependencies branch November 12, 2013 14:50
@seanh
Copy link
Contributor

seanh commented Dec 3, 2013

@vitorbaptista Do you know what the process should be if I just want to add a new dependency, and not upgrade other ones? I guess I could add foo==1.0.0 to both requirements.in and requirements.txt (editing requirements.txt directly)

@vitorbaptista
Copy link
Contributor

@seanh We should never edit requirements.txt directly. To add a new
requirement, we should add it into requirements.in, then do the same
process as when upgrading the requirements (install, freeze to
requirements.txt, etc.).

It's needed to guarantee that we get not only the direct dependency, but
everything that it adds.

When following this process, we might have problems to avoid upgrading
other dependencies by mistake. It might be quite cumbersome but, while we
don't find a good tool to help us, I don't see a better way :/

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.

None yet

3 participants