-
Notifications
You must be signed in to change notification settings - Fork 47
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
Default queue name to request URL if not provided #2
base: master
Are you sure you want to change the base?
Conversation
Thanks for sending this in! I think that providing a default could make it even easier to use, and the URL seems reasonable. What do you think about also tacking on the
Which would be called similarly to your pull request, like this:
|
Hey Brian, Thanks for the feedback. I like the idea of an overridable default queue name function even better. I can modify my PR to include that. However, I'm not sure about including the method in the queue name by default. Maybe I'm missing something, but I don't understand why one would want to queue GET requests to a specific resource except to avoid fetching the resource before pending updates are processed. If that is the case, then including the method in the queue name would actually break this behaviour. Is there another use case for queued GET requests that I am not thinking about? |
The idea was to prevent unintentional queuing. If you specifically didn't want a GET request to be queued, you would be using That said, I see where you are coming from - it seems like a weird use case where the you would want GETs to be queued separately from a POST. Did you rely on the queue including GET requests in your implementations, or were you only using it for POSTs? My reasoning was to prevent a confusing issue where you aren't expecting something to get put in the same queue as your other requests. I think that could be caused by a poorly designed API that shared a URL and overloaded the action that happens on it based on a value passed up, for instance. See a made up example below. If you are doing this though, it is likely that no default queue names will cut it even if we included the method.
|
I'm OK with not including method if you feel strongly about it - I haven't used the default queue behavior, so it was just speculating that Do you have any thoughts about handling |
I would expect queueing GET requests to be rather rare. I haven't used queued GET requests in any of my apps. The advantage you get from queuing POSTs and PUTs is that you avoid issues with out of order updates. I can see that queued GETs might be useful if you were waiting on some POST or PUT to complete, but then again you would probably do that within the success callback for the appropriate update request. In your example above, I would argue that using Anyway, I agree with your point that badly designed APIs will unlikely be able to take advantage of default queue names, regardless of how the are generated. As a result I think it makes sense to have the default cater to nice APIs and just base the queue name on the request URL. If you are okay with this I will add some code to the PR to reflect the new default queue name function. I will also add some documentation. As for the question as to what should be done about |
Sounds like a good plan. Let's just go with URL for now, and if you can add some documentation about it that would be great. I think |
Okay. I'll update the PR shortly. |
This is a great plugin. I've used it on a number of projects and noticed that in all but one case I used the request URL as the queue name. To me, this seems like a reasonable default for scoping requests.
This simple patch does just that. If no queue name is provided, it uses the request URL. Now adding support for queued requests is as simple as including the library and changing calls from
$.ajax
to$.ajaxq
.