civisanalytics / civis-python

Civis API Python Client
BSD 3-Clause "New" or "Revised" License
34 stars 26 forks source link

[CIVP-17567] Check if polling result is None before checking its state #289

Closed hinnefe2 closed 5 years ago

hinnefe2 commented 5 years ago

This PR adds a check to civis.polling._ResultPollingThread.run() to make sure the result returned by the polling function is not None before attempting to access the result's state attribute.

According to the discussion in #149 the None return value is the result of intermittent connectivity errors. The suggested workaround was to rerun the script, but that's annoying when the script takes a long time to run. With this change a None response gets treated the same as a response which has a non-DONE state.

stephen-hoover commented 5 years ago

Thank you for adding this! Please also add a line to the changelog describing the change. This would go under "Fixed".

hinnefe2 commented 5 years ago

I looked into what could be returning Nones a little bit.

The path I followed was resources._resources.create_method() -> base.Endpoint._call_api -> response.convert_response_data_type which lands here.

The only I thing I can thing of is maybe the assert is getting removed during compilation (which happens when you optimize the compilation) and the request is returning a response type not in snake, raw, pandas in which case none of the if blocks trigger. I'm not at all confident I understand the flow of how the endpoint functions get generated though so this could be a red herring.

Anyway, I'll merge this PR for now.