Closed elsander closed 4 years ago
@mheilman Okay, ready for another round of review.
@mheilman ready for another round!
I used the newline to match the formatting in the CivisFuture
docstring itself, although it looks like we then don't use it in ContainerFuture
, so not consistent throughout. I can update it in CivisFuture as well.
Ready for re-approval @mheilman
This PR adds a new method to
CivisFuture
objects,.outputs()
, which blocks on job completion and returns the run outputs. I kept it as minimal as possible to make it generalizeable and easy to maintain, while still making it easy to maintain.I tested the method manually on succeeded and failed container scripts using both
CivisFuture
andContainerFuture
, and also tested on a successful sql script. It's always challenging to add meaningful unit tests to the API client, so I'm happy for feedback on the unit testing I added.