Closed Bug 1253112 Opened 4 years ago Closed 3 years ago
OS] Upload bookmark records atomically
48 bytes, text/x-github-pull-request
|Details | Review|
This is the iOS equivalent of Bug 1253051. See that bug for details.
Relevant API documentation: https://github.com/mozilla-services/docs/pull/60/files
I can take a stab at this. Putting :rnewman as a mentor :)
Assignee: nobody → sleroux
Attachment #8773442 - Flags: feedback?(rnewman) → feedback+
I don't understand swift, but one thing that stands out: The way the spec is written, we can't know the batch semantics of the server until we make the first post (and thus have already enqueued a number of records). It's not clear this is handled here - or maybe the intent was to deduce the server semantics from the result of the info/configuration endpoint and fail if we thought the server was going to give us a batch but didn't? Best I can tell this patch doesn't hit info/configuration at all, nor use the batch limits it returns, so maybe that will become clearer in later versions of the patch?
Summary: Upload bookmark records atomically → [iOS] Upload bookmark records atomically
On iOS we don't support custom servers, so we don't have to worry too much about non-supported batch operations; see Bug 1254402 for using i/conf.
Whiteboard: [data-integrity] → [data-integrity][MobileAS s1.1]
(In reply to Richard Newman [:rnewman] from comment #5) > On iOS we don't support custom servers, so we don't have to worry too much > about non-supported batch operations; see Bug 1254402 for using i/conf. FWIW, I believe the plan for rolling out the server side of this is "slowly" :) It's tricky to fully simulate a production load, so I believe ops will enable this per-node and disable it if any problems are found (then presumably tweak, rinse and repeat). In practice, this means a patch that assumes batch semantics can only land once the config is enabled for all nodes and ops are confident they will never need to turn it off. (Richard says something similar in bug 1254402 comment 2, but I thought it worth explicitly calling out here)
Whiteboard: [data-integrity][MobileAS s1.1] → [sync-atomic][MobileAS s1.1]
Whiteboard: [sync-atomic][MobileAS s1.1] → [sync-atomic][MobileAS s1.2]
Whiteboard: [sync-atomic][MobileAS s1.2] → [sync-atomic][MobileAS s1.3]
Whiteboard: [sync-atomic][MobileAS s1.3] → [sync-atomic][MobileAS]
You need to log in before you can comment on or make changes to this bug.