#metabrainz

/

      • HenryG has quit
      • HenryG joined the channel
      • thomasross has quit
      • sumedh joined the channel
      • sumedh has quit
      • texke has quit
      • texke joined the channel
      • _lucifer
        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.
      • BrainzBot
        LB-849: Should we require users to have a confirmed email before creating an LB account? https://tickets.metabrainz.org/browse/LB-849
      • _lucifer
        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)
      • ruaok
        moooin!
      • Mr_Monkey: zas: alastairp: Freso: yvanzo: invoices please!
      • nelgin
        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.
      • ROpdebee joined the channel
      • BrainzGit
        [brainzutils-python] alastair merged pull request #61 (master…redis-sets): Add redis sets to brainzutils.cache https://github.com/metabrainz/brainzutils-pytho...
      • [brainzutils-python] alastair merged pull request #58 (master…bu24): BU-24: Change function definitions in artist.py, release.py in musicbrainz_db module https://github.com/metabrainz/brainzutils-pytho...
      • alastairp
        _lucifer: why 14 days?
      • this is the login period?
      • _lucifer
        the nightly job runs every 14 days. in case the user does not manually logout and login, we depend on the nightly job to update the email in LB.
      • alastairp
        if it runs every 14 days then it's a biweekly job, not a nightly job... why can't we run it once a day?
      • _lucifer
        oh! i confused nighly with a fortnight 😓
      • yeah, sure we can run that every day.
      • alastairp
        ! ok
      • I guess "nightly" doesn't make much sense with a website used by people all over the world, maintained by people in various different timezones
      • _lucifer
        yeah.
      • alastairp
      • to fix conflicts
      • _lucifer
        yes just a few mins ago
      • alastairp
        I think I fixed them myself and merged to master
      • that's why it didn't close the PR properly
      • _lucifer
        oh!
      • alastairp
        wait, maybe I didn't
      • ah, I'm in the middle of the merge!
      • BrainzGit
        [brainzutils-python] alastair merged pull request #64 (master…cache-misc): Miscellaneous pending cache changes https://github.com/metabrainz/brainzutils-pytho...
      • alastairp
        there it is
      • thanks
      • _lucifer
        let's release BU 2.0!
      • alastairp
        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.
      • !m alastairp
      • BrainzBot
        You're doing good work, alastairp!
      • alastairp
        _lucifer: does this have to be merged to test it?