Add Django 1.11 compatibility, and other modernization

[schbetsy]

• Add Django 1.11 compatibility
• Upgrade requests to 2.18.X, to match cfgov-refresh
• Use Django migrations to replace south for handling db migrations
• Use Tox to test across Django versions 1.8 and 1.11
• Put requirements in setup.py under install_requires and testing_extras
• Rename the default testing database from oah to oah.sqlite3, for clarity

This required upgrading djangorestframework to 3.6.3 or higher.

[schbetsy]

@chosak I was getting errors from the migration commands until I ran pip install psycopg2-binary in my environment. Is that a concern?

[coveralls]

Coverage remained the same at 99.794% when pulling 927adbd on schbetsy:upgrade-everything into fffe6d6 on cfpb:master.

[schbetsy]

Should these requirements stay pinned to specific versions, like they were in the requirements.txt, or do we want them to be flexible? Flexible would make sense to me, I'm just not sure how much variance to allow.

[schbetsy]

All tests passed when I tested over 2.18.x and 2.19.x.

[chosak]

I get some errors when running locally with Django 1.11 in standalone mode, for example at http://localhost:8000/oah-api/rates/rate-checker I get

TemplateDoesNotExist at /oah-api/rates/rate-checker: rest_framework/api.html

I'm wondering if this is due to a missing TEMPLATES section in settings_for_testing.py. In Django 1.10 it looks like this is now required. See for example the testing settings in the retirement project; if I add something like that here, I can get past the above error, but then I get a new one:

TypeError at /oah-api/rates/rate-checker: context must be a dict rather than RequestContext.

This comes from inside of the rest_framework package. Both this app and cfgov-refresh have djangorestframework==3.1.3, but Django 1.10 support isn't added to DRF until 3.4.0. If I try upgrading locally I start getting additional errors:

NotImplementedError at /oah-api/rates/rate-checker: `request.QUERY_PARAMS` has been deprecated in favor of `request.query_params` since version 3.0, and has been fully removed as of version 3.2.

from this line of code. It looks like if we upgrade these views will need to be modified.

---

I was getting errors from the migration commands until I ran pip install psycopg2-binary in my environment. Is that a concern?

Do you have DATABASE_URL set to a Postgres database URL? That library is needed when testing against Postgres. When @richaagarwal and I were adding PG compatibility to the satellite repos in #125, we added the ability to set a custom test database with that environment variable, as it's the default one used by dj-database-url. In cfgov-refresh we have since changed that to TEST_DATABASE_URL (in cfgov-refresh#4075) to make it easier to use different test/development databases. We could consider making that change on the satellite apps as well.

Relatedly, something else we could consider for the satellite apps is adding/changing the default Travis test database to Postgres, like we have done in cfgov-refresh. That might be out of scope for this PR, though.

---

How would you feel about taking this opportunity to rename the default Sqlite database filename? Right now it defaults to simply oah, which is a non-intuitive filename and a bit confusing when you see it on your filesystem. What if instead we renamed it to something like oah.sqlite3?

---

Should these requirements stay pinned to specific versions, like they were in the requirements.txt, or do we want them to be flexible?

That's a good question. Ideally they'd be flexible to support any version that works. Without real testing to determine what that version should be, it might be best to leave them as is.

[chosak]

I believe that this export statement is unnecessary because manage.py defaults the environment variable this way.

[schbetsy]

@chosak Thanks for your help and patience. This is ready for review again.

[chosak]

👍

To successfully test this with cfgov-refresh, I added --fake-initial to the refresh database script here.

FWIW, I tried manually setting the requirement in cfgov-refresh to this branch and running drama-free-django to generate a build. It packages DRF 3.1.3 because the requirements in cfgov-refresh overwrite whatever the satellite apps specify. You can reproduce the same if you pip install this branch into your cfgov-refresh and then pip install requirements/local.txt on top of it. If you do that, you end up running DRF 3.1.3 against this PR. Things seem to work fine for me doing that; so perhaps the DRF upgrade is only needed if/when Django is bumped to 1.11? Either way, I wonder if we should make a separate task to bump cf.gov DRF in isolation to confirm that wouldn't break anything else.

[schbetsy]

Correct. This app works fine on drf 3.1.3 when it uses django 1.8 (that's what it was on before this PR). For django 1.11, it requires drf 3.6.3 or higher.

I'm not sure what you're suggesting. Should I not be specifying a version of drf in setup.py since it will be ignored anyway? Should I make it a version range (e.g. djangorestframework>=3.1.3) and let it pull in the version it thinks it needs?

Or are you saying I should merge this PR as-is and make any necessary changes to cfgov-refresh?

[chosak]

Sorry for being unclear: this PR is great as-is.

To me it seems like it should cause a conflict for cfgov-refresh and satellite apps to have different dependencies, but as it happens, it doesn't, and cfgov-refresh overrides based on how the build/deploy process happens. That means that right now, cfgov-refresh's 3.1.3 will be used in builds, even though, after this PR, oah-api will be requiring 3.6.3.

I was suggesting that we could bump the version of oah-api in cfgov-refresh and leave DRF alone there (although I do think as part of that bump we should add --fake-initial to the refresh script). We can tackle a cfgov-refresh DRF upgrade separately.