775395 - Remove index_above and index_below in sync2.0?

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Sync, defect)

Description

[Ryan Kelly [:rfkelly]]

Sync2.0 currently lets you filter BSOs based on their sortindex, using index_above and index_below.  It's not clear to me how or why these parameters would be used by the client.  Are they currently in use?

The server currently does not index on the "sortindex" field so they are not terribly efficient as a filter.  If we have no plans to use them, I suggest removing them to simplify the API.

Comment 1

[Toby Elliott [:telliott]]

historically, these were so that devices with low connectivity could get the most important items first. Give me the top 100 history items. Now the next 100, etc.

Comment 2

[Richard Newman [:rnewman]]

(In reply to Toby Elliott [:telliott] from comment #1)
> historically, these were so that devices with low connectivity could get the
> most important items first. Give me the top 100 history items. Now the next
> 100, etc.

Surely that would be achieved via order=index&limit=100, not index_above?

Comment 3

[Toby Elliott [:telliott]]

It can be. Back then, we cared more about only getting items over a certain threshold. 

I suspect we can drop it.

Comment 4

[Ryan Kelly [:rfkelly]]

(In reply to Richard Newman [:rnewman] from comment #2)
> (In reply to Toby Elliott [:telliott] from comment #1)
> > historically, these were so that devices with low connectivity could get the
> > most important items first. Give me the top 100 history items. Now the next
> > 100, etc.
> Surely that would be achieved via order=index&limit=100, not index_above?

It would be, if add offset back into the API.  Marking the dependency on that bug.

Comment 5

[Ryan Kelly [:rfkelly]]

Docs updated to reflect removal: https://github.com/mozilla-services/docs/commit/303c5bb3b96589b76970dac39e7bcd8da8eb3ec7

Comment 6

[Ryan Kelly [:rfkelly]]

Attachment: patch removing index_above and index_below

Comment 7

[Toby Elliott [:telliott]]

Comment on attachment 645563 [details] [diff] [review]
patch removing index_above and index_below

I am amused by the if statement that magically went away, but the rest looks good.

Comment 8

[Ryan Kelly [:rfkelly]]

https://github.com/mozilla-services/server-syncstorage/commit/ecdce2b2a9d084baed560831332e2ceaed49337d

Comment 9

[James Bonacci [:jbonacci]]

Is this to be part of some future deployment? Or something I can just verify and close?

Comment 10

[Toby Elliott [:telliott]]

This is long-gone. Close away.