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

[WIP - Do not commit] Initial commit - NIBRS Agency API #507

Merged
merged 11 commits into from
May 19, 2017

Conversation

cacraig
Copy link
Contributor

@cacraig cacraig commented May 15, 2017

  • Added agency aggregations to Mat views
  • Compacted/re-orged Materialized views SQL
  • Modified APIs to give null for missing fields

… Modified APIs to give null for missing fields
@cacraig
Copy link
Contributor Author

cacraig commented May 16, 2017

@harrisj - When you get a chance, can you take a look at this? - I needed to disable the swagger doc verification for the tests, and some parts of the unit tests because of it failing with null fields. Since null is now returned when a data point is missing/0.

@harrisj
Copy link
Contributor

harrisj commented May 17, 2017

Okay, I brought back the Swagger tests, but the test_cdemodels tests run really slowly on my machine for some reason...

@cacraig
Copy link
Contributor Author

cacraig commented May 17, 2017

Thanks @harrisj - The slowness is caused by the JOINING on some pretty big tables - which would be compounded on production. I modified the queries to scan our denormalized/partitioned tables (only one year) instead. It should be a whole lot faster now. We can further tweak it (if necessary) by creating some views to join on that have these distinct values.

age_num, sex_code, and resident_status_code don't have their own linking tables, so I am using the nibrs_victim_denorm table to get their distinct values.

@cacraig
Copy link
Contributor Author

cacraig commented May 18, 2017

Assuming this passes tests - this is done, and ready for another review. The DB changes are live as well. @harrisj

@harrisj
Copy link
Contributor

harrisj commented May 19, 2017

I would ultimately like to fix some of those tests you had to weaken, but this is generally looking good so I don't want to hold things up too much longer, especially since there is high need for this in the sprint. Is this ready to be merged?

@cacraig
Copy link
Contributor Author

cacraig commented May 19, 2017

I'd like to get this deployed, so I can evaluate performance issues against the main DB - I'll submit a followup PR with better tests/docs + fixes for performance problems that may arise. Anywho, it is ready to go @harrisj - Thanks.

@harrisj
Copy link
Contributor

harrisj commented May 19, 2017

I can help with the tests later. Merging.

@harrisj harrisj merged commit 37026d1 into master May 19, 2017
@harrisj harrisj deleted the nibrs-agency-api branch May 19, 2017 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants