Closed Bug 1151077 Opened 5 years ago Closed 5 years ago
Desktop reading list sync module should batch its POST /batch requests
Almost forgot about this, but the list of requests in POST /batch is "limited to 25 by default", which means we have to batch our batches. I asked Richard about it a week or two ago and he said indeed that's right but they aren't limiting batches on Android yet. http://readinglist.readthedocs.org/en/latest/api/batch.html
It sounds a little like this might break sync for users who add more than 25 items before initially syncing, so bumping priority and "affected" flags accordingly.
yodawg.png (In reply to Drew Willcoxon :adw from comment #0) > I asked Richard about it a week or two ago and he said indeed that's right but > they aren't limiting batches on Android yet. Actually, we aren't using batching *at all*. We make one HTTP request per item. ("If you're not embarrassed by your v1, you're not shipping soon enough")
I manually verified that this works by adding a hundred new items and syncing. I modified the scheduler to not add its reading list listener so that it didn't sync when I added the items so that I could fire off one sync with all the new items at once. I also tested without that scheduler modification, and that worked fine too.
Attachment #8589271 - Flags: review?(mhammond)
Attachment #8589271 - Flags: review?(mhammond) → review+
Drew, just a reminder we need to uplift this.
Comment on attachment 8589271 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: Desktop reading list sync, bug 1131416 [User impact if declined]: Users with more than 25 items to sync at one time likely will not be able to properly sync [Describe test coverage new/current, TreeHerder]: Manual testing with production reading list server [Risks and why]: Low risk, fairly simple patch [String/UUID change made/needed]: None
Comment on attachment 8589271 [details] [diff] [review] patch Approving for uplift to aurora.
Attachment #8589271 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8589271 [details] [diff] [review] patch We want this feature to work as best as possible, taking it for 38. Should be in 38 beta 4
Attachment #8589271 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.