984731 - Accept DELETE requests at the root service endpoint

Status: VERIFIED FIXED

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

Description

[Ryan Kelly [:rfkelly]]

Attachment: sync15-delete-at-endpoint-url.diff

The tokenserver cleanup script in Bug 984297 current needs to know some internal details of the syncstorage API in order to send it a "delete all data for this user" request.  ISTM this would be neater if it could assume all services accept "DELETE <endpoint-url>" as such a command.  Here's what it would take to make syncstorage accept such a request.

Comment 1

[Ryan Kelly [:rfkelly]]

Attachment: sync15-docs-delete-at-endpoint-url.diff

And a corresponding docs change.  It's not a big deal but this seems neater to me than calling "DELETE /storage" from the tokenserver scripts.

Comment 2

[Toby Elliott [:telliott]]

Comment on attachment 8392674 [details] [diff] [review]
sync15-docs-delete-at-endpoint-url.diff

Makes sense. You delete all the stuff under /info as well.

It's a little odd that you can DELETE the root, but not do anything else to it, but that's mostly aesthetic.

Comment 3

[Toby Elliott [:telliott]]

Comment on attachment 8392673 [details] [diff] [review]
sync15-delete-at-endpoint-url.diff

Review of attachment 8392673 [details] [diff] [review]:
-----------------------------------------------------------------

::: syncstorage/views/__init__.py
@@ +177,5 @@
> +    storage = request.validated["storage"]
> +    storage.delete_storage(request.validated["userid"])
> +    return {}
> +
> +

Are we going to remove the corresponding DELETE /storage? The test implies we should, but there's no removal here.

Comment 4

[Toby Elliott [:telliott]]

Comment on attachment 8392673 [details] [diff] [review]
sync15-delete-at-endpoint-url.diff

Review of attachment 8392673 [details] [diff] [review]:
-----------------------------------------------------------------

I guess we probably shouldn't for now, come to think of it, since the client will still be deleting to the other endpoint. Adding Nick and Richard to note it as something we should deprecate.

Comment 5

[Richard Newman [:rnewman]]

I think there's a difference between "delete my account" and "wipe my storage", so I see no reason to merge the two.

Comment 6

[Toby Elliott [:telliott]]

From the perspective of the storage server, the calls are identical, though.

Comment 7

[Ryan Kelly [:rfkelly]]

> Are we going to remove the corresponding DELETE /storage? 
> The test implies we should, but there's no removal here.

Nope, we keep it for b/w compat, and there are other tests (unchanged by this patch) that still exercise it.

https://github.com/mozilla-services/docs/commit/3a6745b332d7739905ca85c4d4852a2f3529510a
https://github.com/mozilla-services/server-syncstorage/commit/986ae66ddb394c3c2097eec7580d4692b8efe92a

Comment 8

[James Bonacci [:jbonacci]]

See also https://github.com/mozilla-services/server-syncstorage/issues/16
To be verified together with next deploy to Sync Stage.

Comment 9

[James Bonacci [:jbonacci]]

I think we are all good here.
Super clean Sync load tests last week and this week.