Processing results are a list #195
Conversation
This was a copy/paste mistake.
In this commit we convert `FacilityListItem.processing_results` from an object to a list. `processing_results` now acts as a historical log of processing events and allows the same processing action to potentially be retried in the case of a failure without losing and previously logged information.
jwalgran
changed the title
|
I did not know about |
|
Processed List 2, then inspected the database for errors and found a log of events in the SELECT jsonb_pretty(processing_results)
FROM api_facilitylistitem
WHERE facility_list_id = 2
AND status = 'ERROR'
LIMIT 1;[
{
"error": false,
"action": "parse",
"started_at": "2019-02-11 20:13:33.446086",
"finished_at": "2019-02-11 20:13:33.447321"
},
{
"error": true,
"trace": "Traceback (most recent call last):\n File \"/usr/local/src/api/processing.py\", line 72, in geocode_facility_list_item\n data = geocode_address(item.address, item.country_code)\n File \"/usr/local/src/api/geocoding.py\", line 42, in geocode_address\n raise ValueError(\"No results were found\")\nValueError: No results were found\n",
"action": "geocode",
"message": "No results were found",
"started_at": "2019-02-11 20:14:25.191696",
"finished_at": "2019-02-11 20:14:25.285854"
}
]where the traceback can be formatted to look like: |
|
While I get one of these for every item in the list. If I run |
|
Can't find a clear code change that would indicate why this error is happening. Quick googling suggests it being a memory leak / garbage collection / initialization order issue. |
|
This seems related: https://code.djangoproject.com/ticket/29543. Was fixed in django/django#10130, but isn't in a release yet, only pre-releases 2.2b1 and 2.2a1. |
|
Thanks for doing some research into that error on match. I'm skeptical that it isn't my fault, since both stack traces end in my code, although the traces are pretty confusing. I ran into this exception when I was converting the code and had a lingering item like item.processing_results[ProcessingAction.GEOCODE]Which was clearly tring to access what is now an array as if it was an object. That's not what is happening in the stack traces you posted, though. I'll do some more detective work. |
|
I could not reproduce the exceptions you ran into until... I reset my database. Then: Something with the fixture data is breaking an expectation in the code. I should be able to track it down. |
|
I was able to reproduce the It turns out that our inconsistent fixture data led us into this problem. The fixtures already have some My plan is to just make the fixtures consistent to avoid the integrity error and wait on a new Django release for the fix to the CPointer bug. |
We were previously creating a `Facility` for every `FacilityListItem` but we were only creating a `FacilityMatch` for a random third of the items and not setting the status of the `FacilityListItem`s correctly. In this commit we have updated the `makefixtures` command and the affected fixtures so that the data is consistent.
|
I pushed up a revision to the fixtures and the fixture generator that make the loaded data consistent and avoids the unhandled |
|
Great work! |
|
Thanks for finding and puzzling through that exception with me. |
Overview
Converts
FacilityListItem.processing_resultsfrom an object to a list.processing_resultsnow acts as a historical log of processing events and allows the same processing action to potentially be retried in the case of a failure without losing and previously logged information.Connects #193
Demo
Testing Instructions
./scripts/manage reset_schema --noinput./scripts/manage migrate./scripts/manage loadfixtures./scripts/manage batch_process -a parse -l 2./scripts/manage batch_process -a geocode -l 2./scripts/manage batch_process -a match -l 2./scripts/manage dbshellselect jsonb_pretty(processing_results) from api_facilitylistitem where facility_list_id = 2 and row_index = 0;