RQ Job Timeout Handling #223
Conversation
- Always close the temp file on exception handling. - Added handling for the rq job timeout.
ckanext/xloader/jobs.py
Outdated
| @@ -222,6 +222,11 @@ def tabulator_load(): | |||
| resource_id=resource['id'], | |||
| mimetype=resource.get('format'), | |||
| logger=logger) | |||
| except rq_timeouts.JobTimeoutException as e: | |||
There was a problem hiding this comment.
Wouldn't it be better to put this further out, eg line 256?
There was a problem hiding this comment.
@ThrawnCA moved out to wrap the tabulator_load method call with the try/catch. Lemme know if that is actually the right place. Thanks!
There was a problem hiding this comment.
It's a start, but I think line 267 would be a better place.
- Moved try/catch for `tabulator_load` rq timeout up.
| tabulator_load() | ||
| try: | ||
| tabulator_load() | ||
| except rq_timeouts.JobTimeoutException as e: |
There was a problem hiding this comment.
This isn't catching timeouts from the tabulator_load on line 266. Perhaps move this block to the outer catch, line 267?
|
@JVickery-TBS i'd like the idea but with the tests failing, I can't look at merging this. Also unsure about what @ThrawnCA picked up on another path that is not caught correctly. |
|
Hi @JVickery-TBS , I'd like to incorperate this into xloader. Are you able to rebase to master so that we have the tests fixed. Regards, |
fix(logic): close temp file on rq timeout;