search

civisanalytics / civis-python

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

Future propagates error #418

Closed elsander closed 3 years ago

elsander commented 3 years ago

Note: this PR is a copy of #416 , transferred on to a branch since I likely won't be able to merge before my last day. CC @ggarcia-civis.

Notes from previous PR: The Civis API currently returns 'error': None when a Python/R/container script fails, so API client users don't get any information about what caused the failure. CivisML works around this by returning error logs to the user in these cases. This PR moves this behavior from ModelPipeline to ContainerFuture so that users can get more informative errors when running jobs outside of CivisML as well.

This is something of a workaround, especially since R client users won't get the benefits. Longer-term, we'd like to get more informative errors directly from the API.

elsander commented 3 years ago

@stephen-hoover and @mheilman-- I tagged you for review as a heads up, but I haven't made the changes from the comments in the previous PR yet, so no action needed.

stephen-hoover commented 3 years ago

@elsander , I suggest that we continue on #416 and close this PR. We can continue using the branch off of your fork (which you should still have access to), and remove this branch.