chop-dbhi / avocado

Metadata APIs for Django
http://avocado.harvest.io
Other
41 stars 10 forks source link

[wip] Fix async results #324

Closed bruth closed 8 years ago

bruth commented 8 years ago

@murphyke @aaron0browne I will squash, but I was able to rework the code so the parts necessary to produce the SQL and params are done prior to setting up the async job. That prevents needing to pass the request or other user data when the query actually executes in the async process.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 81.319% when pulling 2025309645be8bfca37db3ae6a300f305c5b7c38 on better-async-results into ca4439228b66225e3d1667a58b3c22c036a54608 on master.

murphyke commented 8 years ago

@bruth Great, thanks! Prior to the async feature, all invocations ofget_queryset had request=request passed, which was done to enable PCGC's use of authorization in its custom query processor. Presumably this feature went by the wayside because it was not obvious that this request argument had any value. I see the request object being put into the query_options parameter of get_result_rows but not for async_get_result_rows, and in any case, it is not used.

Some questions:

Just to test things out, I have a branch of Serrano that passes request=request to *get_result_rows and an Avocado branch based on yours that passes **kwargs to get_queryset. This works -- without the cancellation error I was seeing before.

bruth commented 8 years ago

Just to test things out, I have a branch of Serrano that passes request=request to *get_result_rows and an Avocado branch based on yours that passes **kwargs to get_queryset. This works -- without the cancellation error I was seeing before.

Lets go this route and promote request to be a documented parameter for get_results_rows.