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.
MRiddickW has quit
sumedh joined the channel
sumedh has quit
sumedh joined the channel
alastairp
_lucifer: good morning
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
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
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? :)
*send
So there does seem to be a problem with replication slowing to a crawn.
3:19am search.index = 1347
3:29am search.index = 1345
ruaok
outsidecontext: zas: have you looked at the picard gsoc proposal to see if you wish to accept it?
pm me please
_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.
the i've added my email button sounds good. yeah, i don't think we should allow users to change email.
(change email in LB)
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
_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.
alastairp
this is the same issue as users changing their username, we should solve it in the same way
can we not do a lookup each time the user signs in?
_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.
currently, we just use the email to in the listens importer to inform users of a failure i think.
alastairp
yeah, right. so this is something that we'll have to sort out for username changes too
_lucifer
yeah, i was thinking of maybe internal hook for mb to notify that the username has changed to the app?
somewhat like the sitewide delete-user workflow.
alastairp
there should already be hooks for notifying if an account is deleted, so that might be possible
_lucifer
yes right.
alastairp
or alternatively, this could trigger a logout on other sites
anyway, also something to consider when moving user accounts to metabrainz
_lucifer
yeah, that works as well.
yes right.
alastairp
should we have the same system, or should all sites just access the database directly for user data?
_lucifer
currently, we access directly i think.
reosarevok
FWIW: when changing a username in MB, I always tell the user to log out and in again
_lucifer
direct access seems fine to me.
reosarevok
But of course, they can change their email without asking us, so that's harder :p
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?
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)
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
reosarevok
alastairp: yeah, in most cases (for now at least) the change will be initiated from the MB side
In the end I hope we would all be calling the meb DB :p
Once we have it
_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.
I think both of these makes sense.
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
_lucifer: sorry, I don't understand what you mean in your second sentence
reosarevok
That would make sense, yeah
But all other projects would still need to know about the change too, not only the one the user came in from :)
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
reosarevok
Sure
_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.
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
let's stop talking about the future and work out a way to do this currently
_lucifer
yes, makes sense.
in the current situation, firstly i am wondering if we gain anything by storing the email in LB?
reosarevok
How often do you need it and what for?
alastairp
right, because as you say we only use it in one place. in that case we could look it up on demand?
_lucifer
yes, precisely.
alastairp
however, we _do_ need some kind of indication that a user has an email - in order to block submissions at some future point
reosarevok
Can't you just store a boolean "has_verified_email"?
_lucifer
right, hence the `has_email` column. even if the user changes the email we do not need to update it.
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)
So I think you could get away with rarely checking that one at all
On login might even be fine?
_lucifer
is it possible to remove the email?
alastairp
but we still have to update when a user adds an email address (if they don't currently have one)
reosarevok
Yes, you can blank your email
_lucifer
for that we'll add a button as you suggested earlier.
reosarevok
IIRC anyway
alastairp
reosarevok: yeah, the inital plan was to do this on login
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
alastairp
one other option is that we could periodically update this with a nightly job
reosarevok
If you only check users currently marked no-email, that might not be a lot
Checking all sounds annoying
_lucifer
nightly job makes sense.
yeah, i just tried, i was able to remove my email from MB.
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
So a fair amount of old emails probably don't resolve at all anyway
alastairp
reosarevok: mostly so that we can email users and say "hey, something went wrong"
reosarevok
I know I get a fair amount of bouncebacks even for autoeditor election emails
alastairp
especially for spotify import at the moment, and at a later stage for being able to contact a user if they're spammy
reosarevok: does MB track bouncing emails? (what service do you use to send emails? hosted email platform or just plain smtp from MB?)
reosarevok
For this, I use a small python script locally :p
But for actual emails from the site, I'm not sure exactly
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
reosarevok
bitmap or zas almost certainly know
Not sure if we do track bounces
zas
we don't
reosarevok
lol. I like it when something consistently breaks, but works fine if I console.log the affected variables first
zas: do we have the possibility to?
(and then say, after X bounces we force you to re-verify)
_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.
CatQuest
[11:43] <reosarevok> (and then say, after X bounces we force you to re-verify)
+1 to this (esp for autoeds)
if like idunno a year passes and no re-verify they could have their privs disabled *temporarily*
I was looking at the old autoeditor elections table. it's disparaging ot see so many that are "deleted editor" :(
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?
_lucifer
alastairp: yeah, that'd be the case.
but now that we are doing it this way. i am fine with storing the email directly as well.
ROpdebee has quit
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
then I'll have lunch, but yes, after that!
_lucifer
sure.
alastairp
_lucifer: are you working on this email-required stuff now? then maybe starting tomororw we can have a discussion about AB?
_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.
alastairp
fine
_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.