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

Prevent polling requests from being queued up #4980

Merged
merged 23 commits into from
Aug 12, 2015

Conversation

alonsogarciapablo
Copy link
Contributor

Fixes #4939.

I've basically extracted some of the polling logic from GeocodingModel and ImportModel into GeocodingModelPoller and ImportModelPoller, which are specifications of a generic Poller. Instead of using an interval to trigger fetch requests periodically, pollers now wait for the previous request to be received before triggering new ones :)

@viddo could please take a look? I'm looking forward for your feedback! Thanks!

@alonsogarciapablo alonsogarciapablo changed the title [WIP] Prevent polling requests from being queued up Prevent polling requests from being queued up Aug 12, 2015
@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

return model.hasFailed() || model.hasCompleted();
},
error: function() {
this.model.trigger("change");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason for using this.model and not just model, like done in condition fn above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I thought about this but then forgot to revisit it. There's no specific reason.

I will also pass model to the error callback so that there's no need to know about the internals of the Poller:

...
error: function(model) {
   model.trigger("change");
}
...

@viddo
Copy link
Contributor

viddo commented Aug 12, 2015

Apart from comments it LGTM 👍

alonsogarciapablo added a commit that referenced this pull request Aug 12, 2015
Prevent polling requests from being queued up
@alonsogarciapablo alonsogarciapablo merged commit ddbfb86 into master Aug 12, 2015
@alonsogarciapablo alonsogarciapablo deleted the 4939-better-polling branch August 12, 2015 15:47
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 this pull request may close these issues.

None yet

3 participants