Sindri-Labs / sindri-python

Contains SDK code for Sindri APIs
MIT License
4 stars 0 forks source link

Separate API Hits from Polling Loops #7

Closed KPreisner closed 7 months ago

KPreisner commented 7 months ago

Separate API Hits from Polling Loops

This PR splits the internals of the following methods into multiple private methods so they can be used in a non-blocking fashion.

Prior to this PR, the only way to create a circuit or prove a circuit with this SDK was to call the blocking methods that raised the Sindri.APIError. Now, an internal developer can call the single endpoints for create/detail and check their return status codes individually. They can also wrap them in their own polling/async techniques if desired.

KPreisner commented 7 months ago

The relocated code looks okay in general. However, considering the comment

Now, a developer can call the single endpoints for create/detail and wrap them in their own polling/async techniques.

I wonder if the new "atomic" methods should be public, not private? And if that's the case we might want to reconsider the method naming scheme. I'd appreciate a bit more color around the need/desire for this change. In particular I'd like to see a "new feature" PR template-style description, and a clearer understanding of the audience for this update.

Good catch! I've updated the bottom section of the description to match my intentions with this refactor.

Prior to this PR, the only way to create a circuit or prove a circuit with this SDK was to call the blocking methods that raised the Sindri.APIError. Now, an internal developer can call the single endpoints for create/detail and check their return status codes individually. They can also wrap them in their own polling/async techniques if desired.

In a future PR, we can consider making public versions of the independent "create" methods; independent "detail" methods already exist. I think we need to plan well with the naming of these new public methods so they are not confused with the existing ones that create and poll.