search

civisanalytics / civis-python

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

ContainerFuture propagates error from logs #416

Closed elsander closed 3 years ago

elsander commented 3 years ago

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.

ggarcia-civis commented 3 years ago

@stephen-hoover @mheilman Sorry about the long wait period. I tried to address all of the remaining feedback.

stephen-hoover commented 3 years ago

@ggarcia-civis , I'm unlikely to be able to re-review in a timely fashion, so I will leave this review to @mheilman .

ggarcia-civis commented 3 years ago

Hi @elliothevel, Mike is on PTO this week. Can you pick up on this PR review?