Ajax retry configuration #1762
Ajax retry configuration #1762
Conversation
…Maker. Also changing ajaxRetryInOrderReceived to be a FactoryMaker as it both avoids a var, and can be sensitive to state such as the current request
…send the default to the client if the FactoryMaker returns and Empty
| val ajaxAddMetaAndQueueSortFuncs: FactoryMaker[Box[(JsVar => JsCmd, JsVar => JsCmd)]] = new FactoryMaker[Box[(JsVar => JsCmd, JsVar => JsCmd)]]( () => | ||
| Full((ajaxAddMetaDefaultFunc, ajaxQueueSortDefaultFunc)) | ||
| ) {} | ||
|
|
There was a problem hiding this comment.
I'd love to have some feedback on the design of this setting. To achieve ordered messages, you have to change both of these functions together. I think this will be the most common use case. However, I can imagine where someone may want to change the ajaxAddMeta function, but not ajaxQueueSort. For instance, if they wanted to add some meta data related to ajaxCalcRetryTime since it has access to the meta.
I went around in circles a bit and decided to just get it out here and have some feedback.
There was a problem hiding this comment.
Could we have this FactoryMaker vend some case class instead of a tuple? Would make the code a lot easier to reason about while scanning. And would also avoid client code doing a lot of _1 and _2.
There was a problem hiding this comment.
Absolutely could. I'm just not quite sold on making sure they're coupled together once I noticed that the ajaxAddMeta could conceivably be useful for ajaxCalcRetryTime. Maybe I'm being too open-minded to the world of possibilities which stretches beyond what anybody would actually attempt.
There was a problem hiding this comment.
Ultimately, I'm trying to get the flexibility intended here, while making really easy for a developer to do the right thing.
There was a problem hiding this comment.
So the case here is we want to make it easy for the developer to do what they want while encouraging them to couple these things as well because it's important to consider both at the same time? Am I interpreting that right?
There was a problem hiding this comment.
Right. The ajax queue contains all of the requests wrapped with meta data. Hence ajaxQueueSort() works on those wrappers. If you want to do something different in ajaxQueueSort(), then it's highly likely you'll need to add to that wrapper in ajaxAddMeta().
Furthermore, the out-of-the-box configuration alternative we're offering here is the retry in order, which certainly needs both in order to function correctly. Hence I made the pair a single setting so it's not easy to set it incorrectly.
Another alternative I considered was having these two as separate settings and provide a LiftRules function of type Unit => Unit which sets both correctly for retrying in order. I'm not aware of a prior precedent for this approach in LiftRules, though.
|
👀 |
|
After discussion, we have decided the the same functionality of this PR can be achieve by overriding the ajax call in its entirety. The complication of our JS settings don't seem worth having. |
|
Some more thoughts here so it's written down somewhere public :) I think the goal of So, if we find something you can't customize, we should probably fix |
This PR is to give Lift applications more control over ajax retries. With these changes an application can change the ajax queue's sorting logic and retry timeout calculation.
See this ML thread for some discussion regarding this idea.