Give every object in solitude an e-tag (bug 895172) #118
Conversation
|
|
||
| class ListModelMixin(object): | ||
| """ | ||
| Turns the django-rest-framework mixin into an etag-aware one. |
There was a problem hiding this comment.
instead of copypasta can we inherit and override methods?
There was a problem hiding this comment.
Sadly, the granularity of the DRF doesn't allow to override just a method to deal with etags. I can inherit from the original mixin but the only method will be overridden so I think it's useless.
The support of etags in DRF is discussed in this issue (and a PR rejected by the maintainer)
There was a problem hiding this comment.
Just curious, can the addition and validation of etags not be done by overriding finalize_response and initialize_request respectively, in the respective view classes?
There was a problem hiding this comment.
You need a method that take a request and data (without performing the db request twice) and return a response to be able to use the etag decorator from Django. I preferred to modify the DRF at a higher level than to code my own etag decorator, this way I hope the switch to the future support of etags in DRF will be easier.
|
Looking good so far, ping me when you need another review on this. |
| @@ -3,4 +3,5 @@ DELETE FROM seller_product_bango; | |||
| DELETE FROM seller_product; | |||
| ALTER TABLE seller_product ADD COLUMN external_id varchar(255) NOT NULL; | |||
| ALTER TABLE seller_product ADD UNIQUE (`seller_id`, `external_id`); | |||
| ALTER TABLE `seller_product` ADD COLUMN `epoch_micro` bigint(20) NULL DEFAULT NULL; | |||
There was a problem hiding this comment.
I had to add the column in an older migration otherwise 32-populate-public-id.py fails.
|
@andymckay having a counter indexed on epoch ms makes it easier to debug collisions. Is there any interest in storing the number of time an item is saved? |
|
Should there be collisions? Everything in solitude is run in a transaction, so mysql should do all that work for us. |
|
I just realized that the |
| @@ -1,17 +0,0 @@ | |||
| import uuid | |||
There was a problem hiding this comment.
maybe add a comment to this file now
|
looks ok to me with use of F(), but would ask for @wraithan input too |
|
has been summoned r+wc, looks good to me with @andymckay's comments. |
Give every object in solitude an e-tag (bug 895172)
Still missing:
epoch_microapproachetagUpdateModelMixinis not in use/tested for now)counter+pkas a unique etag