ruaok: alastairp: regarding LB-849, since LB user table does not store emails, we'll need to lookup the MB db everytime to check for whether the user email is present. This is probably fine for displaying a banner on login. but doing so on every call to listen submission endpoint seems not be a good idea.
I was thinking if we should generate a list of users who do not have email associated put them in a separate table or cache. if a submission comes from one of these users we check mb db and update our table/cache accordingly. if the user is not in that table, we can skip the the check.
2021-05-03 12313, 2021
MRiddickW has quit
2021-05-03 12319, 2021
sumedh joined the channel
2021-05-03 12359, 2021
sumedh has quit
2021-05-03 12354, 2021
sumedh joined the channel
2021-05-03 12302, 2021
alastairp
_lucifer: good morning
2021-05-03 12306, 2021
alastairp
we should do email lookup at login and copy it into the users table. then this data will be available in the flask current user object
2021-05-03 12335, 2021
alastairp
you're right that we might need some kind of workflow so that a user can say "I've added my email to musicbrainz now", maybe that can be a button in the profile
2021-05-03 12309, 2021
alastairp
we should also decide if we want to allow users to change their email in LB, or if it should always use the MB one (I'm tending towards not allowing them to change)
Where do I sent my invoice for all the testing and stuff I've been doing on the VM? :)
2021-05-03 12349, 2021
nelgin
*send
2021-05-03 12332, 2021
nelgin
So there does seem to be a problem with replication slowing to a crawn.
2021-05-03 12332, 2021
nelgin
3:19am search.index = 1347
2021-05-03 12332, 2021
nelgin
3:29am search.index = 1345
2021-05-03 12337, 2021
ruaok
outsidecontext: zas: have you looked at the picard gsoc proposal to see if you wish to accept it?
2021-05-03 12356, 2021
ruaok
pm me please
2021-05-03 12316, 2021
_lucifer
alastairp, makes sense. so we add a column `has_email` in the users table and retrieve that during get_user which we have to do anyways in all these scenarios. however, we need to add this check to the submit-listens API endpoint as well so we'll have to prepopulate the field for all users beforehand once.
2021-05-03 12302, 2021
_lucifer
the i've added my email button sounds good. yeah, i don't think we should allow users to change email.
2021-05-03 12338, 2021
_lucifer
(change email in LB)
2021-05-03 12338, 2021
alastairp
_lucifer: we should add a column `email` which contains their actual email address. but yes, we should populate it for all users who do have an email in MB
2021-05-03 12307, 2021
_lucifer
alastairp: there's one issue, with adding the email directly to the users table. if the user changes it in MB, we'll have to add another button say update my email in LB.
2021-05-03 12335, 2021
alastairp
this is the same issue as users changing their username, we should solve it in the same way
2021-05-03 12342, 2021
alastairp
can we not do a lookup each time the user signs in?
2021-05-03 12326, 2021
_lucifer
yes, that's possible. but i've logged in for many days in my browser for the lookup to take place i'd have to logout and login. i assume many others might have to do that as well.
2021-05-03 12308, 2021
_lucifer
currently, we just use the email to in the listens importer to inform users of a failure i think.
2021-05-03 12311, 2021
alastairp
yeah, right. so this is something that we'll have to sort out for username changes too
2021-05-03 12325, 2021
_lucifer
yeah, i was thinking of maybe internal hook for mb to notify that the username has changed to the app?
2021-05-03 12307, 2021
_lucifer
somewhat like the sitewide delete-user workflow.
2021-05-03 12308, 2021
alastairp
there should already be hooks for notifying if an account is deleted, so that might be possible
2021-05-03 12331, 2021
_lucifer
yes right.
2021-05-03 12341, 2021
alastairp
or alternatively, this could trigger a logout on other sites
2021-05-03 12305, 2021
alastairp
anyway, also something to consider when moving user accounts to metabrainz
2021-05-03 12309, 2021
_lucifer
yeah, that works as well.
2021-05-03 12312, 2021
_lucifer
yes right.
2021-05-03 12332, 2021
alastairp
should we have the same system, or should all sites just access the database directly for user data?
2021-05-03 12333, 2021
_lucifer
currently, we access directly i think.
2021-05-03 12332, 2021
reosarevok
FWIW: when changing a username in MB, I always tell the user to log out and in again
2021-05-03 12340, 2021
_lucifer
direct access seems fine to me.
2021-05-03 12345, 2021
reosarevok
But of course, they can change their email without asking us, so that's harder :p
2021-05-03 12306, 2021
alastairp
_lucifer: I mean: should LB just connect straight to the meb database to get username and email, or should we continue with the oauth logout/log in/pull dance?
2021-05-03 12325, 2021
alastairp
reosarevok: right, we could say "to change your username or email click here to go to musicbrainz" and that logs you out and redirects you (though that'll only work in the case that the user uses this workflow to do it)
2021-05-03 12344, 2021
alastairp
otherwise _lucifer's point is valid, we don't want to do an http request to MB each time a user hits any page on LB
2021-05-03 12347, 2021
reosarevok
alastairp: yeah, in most cases (for now at least) the change will be initiated from the MB side
2021-05-03 12357, 2021
reosarevok
In the end I hope we would all be calling the meb DB :p
2021-05-03 12301, 2021
reosarevok
Once we have it
2021-05-03 12316, 2021
_lucifer
alastairp: sorry, still not clear to me. I understand we use OAuth during Sign In to ask the user to grant permissions to us. but then in listens importer we do a direct access of the email ID of the user.
2021-05-03 12352, 2021
_lucifer
I think both of these makes sense.
2021-05-03 12314, 2021
alastairp
reosarevok: I was thinking that it could be a read-only connection. if you want to change your details you'd still get directed to meb to make that change. anyway, this is still all a bit open for discussion
2021-05-03 12323, 2021
alastairp
_lucifer: sorry, I don't understand what you mean in your second sentence
2021-05-03 12333, 2021
reosarevok
That would make sense, yeah
2021-05-03 12357, 2021
reosarevok
But all other projects would still need to know about the change too, not only the one the user came in from :)
2021-05-03 12333, 2021
alastairp
right, but if they used a direct db connection to get user metadata (instead of storing it locally), then it'd be updated as soon as they load it
2021-05-03 12342, 2021
reosarevok
Sure
2021-05-03 12358, 2021
_lucifer
I mean it depends on the context where we need the data. I think that we should do the OAuth dance during sign in. but in all other places i can think of, direct db access is fine.
2021-05-03 12355, 2021
alastairp
sorry, I guess I'm throwing around a bunch of potential future ideas at the same time that we're trying to solve a specific problem with the current infrastructure, and there are some confusions
2021-05-03 12306, 2021
alastairp
let's stop talking about the future and work out a way to do this currently
2021-05-03 12342, 2021
_lucifer
yes, makes sense.
2021-05-03 12328, 2021
_lucifer
in the current situation, firstly i am wondering if we gain anything by storing the email in LB?
2021-05-03 12350, 2021
reosarevok
How often do you need it and what for?
2021-05-03 12357, 2021
alastairp
right, because as you say we only use it in one place. in that case we could look it up on demand?
2021-05-03 12313, 2021
_lucifer
yes, precisely.
2021-05-03 12324, 2021
alastairp
however, we _do_ need some kind of indication that a user has an email - in order to block submissions at some future point
2021-05-03 12342, 2021
reosarevok
Can't you just store a boolean "has_verified_email"?
2021-05-03 12300, 2021
_lucifer
right, hence the `has_email` column. even if the user changes the email we do not need to update it.
2021-05-03 12327, 2021
reosarevok
It's very unlikely that they stop having one entirely (we have less than 1k people IIRC that have edited MB, then removed their email)
2021-05-03 12343, 2021
reosarevok
So I think you could get away with rarely checking that one at all
2021-05-03 12348, 2021
reosarevok
On login might even be fine?
2021-05-03 12351, 2021
_lucifer
is it possible to remove the email?
2021-05-03 12355, 2021
alastairp
but we still have to update when a user adds an email address (if they don't currently have one)
2021-05-03 12358, 2021
reosarevok
Yes, you can blank your email
2021-05-03 12313, 2021
_lucifer
for that we'll add a button as you suggested earlier.
2021-05-03 12314, 2021
reosarevok
IIRC anyway
2021-05-03 12315, 2021
alastairp
reosarevok: yeah, the inital plan was to do this on login
2021-05-03 12354, 2021
reosarevok
But yeah, if you have a message like "you can't do this because you don't have an email" then a "hey, check again!" button might be easiest
2021-05-03 12316, 2021
alastairp
one other option is that we could periodically update this with a nightly job
2021-05-03 12351, 2021
reosarevok
If you only check users currently marked no-email, that might not be a lot
2021-05-03 12355, 2021
reosarevok
Checking all sounds annoying
2021-05-03 12314, 2021
_lucifer
nightly job makes sense.
2021-05-03 12335, 2021
_lucifer
yeah, i just tried, i was able to remove my email from MB.
2021-05-03 12336, 2021
reosarevok
What's the purpose of the email thing, btw? One problem in MB is we don't actually validate the email periodically or anything
2021-05-03 12345, 2021
reosarevok
So a fair amount of old emails probably don't resolve at all anyway
2021-05-03 12353, 2021
alastairp
reosarevok: mostly so that we can email users and say "hey, something went wrong"
2021-05-03 12356, 2021
reosarevok
I know I get a fair amount of bouncebacks even for autoeditor election emails
2021-05-03 12321, 2021
alastairp
especially for spotify import at the moment, and at a later stage for being able to contact a user if they're spammy
2021-05-03 12300, 2021
alastairp
reosarevok: does MB track bouncing emails? (what service do you use to send emails? hosted email platform or just plain smtp from MB?)
2021-05-03 12317, 2021
reosarevok
For this, I use a small python script locally :p
2021-05-03 12335, 2021
reosarevok
But for actual emails from the site, I'm not sure exactly
2021-05-03 12339, 2021
alastairp
for example in freesound we use amazon's SES, and this gives us reports about bouncing emails, and then we mark their email as invalid so that we don't email them again
2021-05-03 12343, 2021
reosarevok
bitmap or zas almost certainly know
2021-05-03 12348, 2021
reosarevok
Not sure if we do track bounces
2021-05-03 12318, 2021
zas
we don't
2021-05-03 12342, 2021
reosarevok
lol. I like it when something consistently breaks, but works fine if I console.log the affected variables first
2021-05-03 12354, 2021
reosarevok
zas: do we have the possibility to?
2021-05-03 12311, 2021
reosarevok
(and then say, after X bounces we force you to re-verify)
2021-05-03 12338, 2021
_lucifer
alastairp: so how about, always check email during login and update the `has_email` column in LB users table. add a nightly job to update this table so that if the user deletes their email and doesn't logout from LB we know. during listens submission and listens importer only check the column in LB users table. in the worst case, we end up accepting listens from users without email for 2 weeks.
2021-05-03 12303, 2021
CatQuest
[11:43] <reosarevok> (and then say, after X bounces we force you to re-verify)
2021-05-03 12303, 2021
CatQuest
+1 to this (esp for autoeds)
2021-05-03 12357, 2021
CatQuest
if like idunno a year passes and no re-verify they could have their privs disabled *temporarily*
2021-05-03 12342, 2021
CatQuest
I was looking at the old autoeditor elections table. it's disparaging ot see so many that are "deleted editor" :(
2021-05-03 12345, 2021
alastairp
_lucifer: right. does this mean that if we have to send an email in the listens importer and has_email is set, we do a query to musicbrainz to get the email address?
2021-05-03 12305, 2021
_lucifer
alastairp: yeah, that'd be the case.
2021-05-03 12319, 2021
_lucifer
but now that we are doing it this way. i am fine with storing the email directly as well.
2021-05-03 12319, 2021
ROpdebee has quit
2021-05-03 12341, 2021
_lucifer
however storing email in LB may still lead to a lag of 14 days update in the worst case. if we want to be sure to use the latest email, we will have to query the MB editor table.
just some changes to the stats PR, I'm applying your changes now
2021-05-03 12356, 2021
alastairp
then I'll have lunch, but yes, after that!
2021-05-03 12304, 2021
_lucifer
sure.
2021-05-03 12338, 2021
alastairp
_lucifer: are you working on this email-required stuff now? then maybe starting tomororw we can have a discussion about AB?
2021-05-03 12320, 2021
_lucifer
alastairp, yeah i am working on that currently. i'll begin AB work from tommorrow. let's see if we can discuss AB later today and draft a plan for the tasks to be done.
2021-05-03 12341, 2021
alastairp
fine
2021-05-03 12300, 2021
_lucifer
ruaok, the spark PR is done and the git repo on newleader is clean now. you can base the similar user tweaks on test-sentry branch for now. i'll merge it into master after review.
