LB-354: Allow deletion of user accounts #406
Conversation
User deletion should delete all referencing rows in stats, api_compat.token and api_compat.session tables.
Also, add relevant test
Also, add relevant tests. Needed to upgrade influxdb-python as the `drop_measurement` method is new.
| NotFound if user isn't present in the database | ||
| """ | ||
|
|
||
| #TODO(param): delete user's listens from Google BigQuery |
There was a problem hiding this comment.
I think this is a job we should push into stats_calculator (might have to rename that to something that indicates generic bigquery operation executor)?
paramsingh
force-pushed
the
allow-user-deletion
branch
from
353d817
to
aa66ce7
Compare
There was a problem hiding this comment.
I think this looks good -- please take a look at my two comments. I think this is ready for merging, but LB-354 is not complete yet since it does not include the bits about the musicbrainz-server callling LB to remove an account. I suppose it might be good to make a separate PR for that. We also need to open a ticket to keep track of the BigQuery data removal.
| @@ -24,7 +24,7 @@ redis == 2.10.3 | |||
| yattag == 1.6.0 | |||
| xmltodict == 0.10.2 | |||
| unittest2 == 1.1.0 | |||
| influxdb == 4.1.0 | |||
| influxdb == 5.0.0 | |||
There was a problem hiding this comment.
Does this upgrade affect any other code?
There was a problem hiding this comment.
Nope, all tests pass. This upgrade was needed because influxclient.drop_measurement was introduced in 5.0.0 (used here)
There was a problem hiding this comment.
Famous last words! I just have a gut-punch feeling every time I see a version updated for anything as critical as a DB. I hope you're right. :)
There was a problem hiding this comment.
well, this isn't a db update per se, just a client update.
There was a problem hiding this comment.
Understood, but it still raises my hackles and makes be really pay attention. Some level of paranoia really is useful. :)
| NotFound if user isn't present in the database | ||
| """ | ||
|
|
||
| #TODO(param): delete user's listens from Google BigQuery |
|
I'm gonna do the musicbrainz api endpoint in another PR, so I think this might be ready to merge, if we're okay with trying out the influxdb-client update. If not, I can drop measurements using direct queries too, but I think using the function should be okay. |
Summary
Problem
Solution
First part of the user deletion stuff. Haven't worked on deleting listens from bq yet, I think
that should be a job ala stats_calculator stuff.