Check finished_processing bool instead of status str when polling for completion #61
Conversation
| self, | ||
| circuit_id: str, | ||
| ) -> str: | ||
| def _get_circuit_status(self, circuit_id: str) -> str: |
There was a problem hiding this comment.
The _get_circuit_status and _get_proof_status "private" methods already used the /status API endpoints. However, they did not return the finished_processing value from the response.
Instead of breaking the functionality of these existing methods, I created the _get_circuit_finished_status and _get_proof_finished_status methods, which are now used by the old _get_circuit_status and _get_proof_status methods.
There was a problem hiding this comment.
I'm uncertain what happened here but it seems very strange that _get_circuit_status() and _get_proof_status private methods even exist. Who calls them? Can you remove them? If you remove them can you simplify the "new" method name?
Please refrain from returning mysterious, magic-integer-indexed tuples. Make a class and return that for clarity. Something like:
class OperationStatus:
def __init__(self, finished: bool, status: str):
self.finished = finished
self.status = status
and
def _get_proof_finished_status(self, proof_id: str) -> OperationStatus:
...
return OperationStatus(finished=finished_processing, status=status)
|
After some Sindri internal conversations, we are holding off on merging this PR in favor of planned refactors of the entire Python SDK. |
closes #57
pytestsucceeds, which confirms the new polling logic is working because it creates circuits and proofs and polls until they are finished processing.sh py_format_code.shsucceeds