Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor API calls to be more robust #80

Closed
ntoll opened this issue Oct 29, 2018 · 1 comment · Fixed by #85
Closed

Refactor API calls to be more robust #80

ntoll opened this issue Oct 29, 2018 · 1 comment · Fixed by #85
Milestone

Comments

@ntoll
Copy link
Contributor

ntoll commented Oct 29, 2018

Now that we have several types of API call happening, we should "brainshare" an implementation that neatly meets our requirements now that they are more clear. This should include:

I suggest the following as a straw man to knock holes in:

  • The class emits a call_finished signal with NO value passed. It's just a signal to say the API call is finished (so it's function is similar to the timeout signal).
  • The raw result of the API call is in self.result. It's assumed that the handler of the call_finished signal will know what to do with that.
  • Any context needed should be passed in as the optional current_object on __init__

This isn't a big change from what we have... in fact I think it simplifies things (fixing #71). However, since all our API calls will go through this (and the associated call_api method on the Client class) then it's important that everyone has an opportunity to kick the tyres before it's implemented (and so we also record the conversation around the design decisions relating to this functionality). Once we get a thumbs up from myself, @redshiftzero and @heartsucker it shouldn't take too long to fix. :-)

@ntoll
Copy link
Contributor Author

ntoll commented Oct 29, 2018

OK... since I'm ploughing through all my GitHub stuff.. I just saw #77 :-) Great minds think alike or fools seldom differ.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants