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

Annual Owning a Home API county-limit update #121

Merged
merged 6 commits into from
Dec 18, 2017

Conversation

higs4281
Copy link
Member

@higs4281 higs4281 commented Dec 16, 2017

At the end of every year, the owning-a-home API's county-limit values
need to be updated. This PR adds data files and an updated fixture
that will load the new limits.

After the OaH API repo is updated and re-imported into cfgov-refresh,
the limit records can be updated in any environment's database with
this manage.py command:

load_county_limits --confirm=y

The update process is documented at [GHE]/OAH/OAH-notes/wiki/Periodic-data-load-procedures.

Regarding Travisty and atonement

This PR also atones for some sins.

  • It removes that nasty initial_data fixture that loaded 6,524 objects with every deploy
  • It drops a convoluted csvkit dependency
  • It fixes a test-killer that was hiding in the views.py file

This repo was plagued by mysterious Travis test failures that didn't happen locally, and it turns out it was my doing.
In responding to a security issue about the API's use of unvalidated query strings, I added a whitelist of state values, but in a foolish way. I added this at the top of views.py:

SAFE_STATE_LIST = (
    list(State.objects.values_list('state_abbr', flat=True)) +
    list(State.objects.values_list('state_fips', flat=True))
    )

This setup allowed the whitelist to be built just once. But since it was evaluated at Django initialization time, that meant that the whitelist would be empty for testing in Travis, where the view would slap a 400 on any state query, behaving as if there were no states, even when a test loaded fixtures or built its own valid state objects.

This is why an initial_data fixture was the only test-loading strategy that worked in Travis, because it got auto-loaded before views.py was initialized.

So here's a toast to Django 1.9+ and the end of initial_data fixtures entirely!

travisty

(noun) A false assertion that the Travis CI service is a travesty.

At the end of every year, the owning-a-home API's county-limit data
needs to be updated. This PR adds the updated data files needed to
load new limits.

Database limit records can then be updated with
```
loaddata countylimit_data.json
```
At the end of every year, the owning-a-home API's county-limit data
needs to be updated. This PR adds data files and an updated fixture
that will load new limits.

After the OaH API repo is updated and re-imported into cfgov-refresh,
the database limit records can then be updated with this manage.py command:

```
load_county_limits --confirm=y
```

This PR also removes the nasty initial_data fixture that loaded with every
deploy, and removes a convoluted dependency on csvkit
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 99.473% when pulling 8909055 on update-countylimit-tables into 897a887 on master.

@higs4281 higs4281 force-pushed the update-countylimit-tables branch 6 times, most recently from db0281c to a4b52eb Compare December 16, 2017 03:35
At the end of every year, the owning-a-home API's county-limit data
need to be updated. This PR adds data files and an updated fixture
that will load new limits.

After the OaH API repo is updated and re-imported into cfgov-refresh,
the database limit records can then be updated in any environment with
this manage.py command:

```
load_county_limits --confirm=y
```

This PR also removes the nasty initial_data fixture that loaded with
every
deploy, and removes a convoluted dependency on csvkit
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 28e7ea2 on update-countylimit-tables into 897a887 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f871196 on update-countylimit-tables into 897a887 on master.

@higs4281 higs4281 force-pushed the update-countylimit-tables branch 2 times, most recently from 3074e2d to 3e9ff8c Compare December 16, 2017 08:03
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 3e9ff8c on update-countylimit-tables into 897a887 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9051630 on update-countylimit-tables into 897a887 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ffbbf2c on update-countylimit-tables into 897a887 on master.

Copy link
Contributor

@hillaryj hillaryj left a comment

Choose a reason for hiding this comment

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

A+ atonement game 🏅

@higs4281 higs4281 merged commit 2a2b9b1 into master Dec 18, 2017
@higs4281 higs4281 deleted the update-countylimit-tables branch December 18, 2017 19:59
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