#metabrainz

/

      • revi has quit
      • HorusHorrendus has quit
      • pprkut has quit
      • revi joined the channel
      • pprkut joined the channel
      • HorusHorrendus joined the channel
      • BrainzGit has quit
      • FichteFoll has quit
      • monkey has quit
      • KassOtsimine has quit
      • Clint has quit
      • mruszczyk has quit
      • Rotab has quit
      • Clint joined the channel
      • Rotab joined the channel
      • KassOtsimine joined the channel
      • monkey joined the channel
      • mruszczyk joined the channel
      • FichteFoll joined the channel
      • yyoung[m]
        Is BrainzBot down?
      • yvanzo
        mo’’in’
      • yes
      • (BrainzGit)
      • BrainzGit joined the channel
      • ruaok
        moin!
      • coffee, yvanzo ?
      • yvanzo
        no thanks :)
      • Etua joined the channel
      • riksucks
        Hey ruaok , if you are free, would you review my PR?
      • ruaok
        will do today, riksucks!
      • riksucks
        thanks ruaok :D
      • lucifer
        ruaok: hi! re spark dumps, the order of the fields has changed, so we should probably rearrange the fields to match older version or increment the version number in SCHEMA file.
      • ruaok
        hi. Hmm, ok, but where are you expecting the artist_mbids column to be inserted? can you paste me the order you're expecting?
      • lucifer
      • ruaok
        oh, right we had them before. sorry. ok, will fix.
      • lucifer
        np :)
      • ruaok
      • were you seeing this order?
      • lucifer
        yes
      • this is the change i made to spark to test these dumps - https://github.com/metabrainz/listenbrainz-serv...
      • ruaok
        nice, I like dictionary value stability in python. :)
      • riksucks: done -- I have some questions
      • yvanzo: do you know if MB editor search is exposed via the MB WS?
      • lucifer
        i tried that yesterday and i think it is not. i was thinking that LB search box could call that WS endpoint and then display results but figured there's another issue that all editors don't have a LB account so probably not the best way to implement it anyway.
      • *that WS endpoint if there was one
      • ruaok
        good point. It just that LIKE queries don't scale well (table scan) and that we already have editor search implemented.
      • maybe if we add a text search index to the LB DB?
      • lucifer
        never used a text search index before so not sure. but no harm in trying it :D
      • ruaok
        I would approve of that approach, but ILIKE will blow up in our faces in the future.
      • lucifer
        let's do that for now as that sounds simple to do. we can deal with scale issues later?
      • maybe when MeB has all users, we can expose an endpoint to search users with an option for Oauth apps and limit the result to those who have approved access to them.
      • ruaok
        the problem with scale issues is that they bite you in the ass when you're busy doing other things.
      • I've done the "we'll worry about the scale issues when they come" before and it ends up being a time-bomb.
      • either we do no search at all, or we do it right. I dont want to build in time-bombs.
      • lucifer
        yeah :/. to be sure, you mean ILIKE and the text search index both have scalability issues or only the former?
      • ruaok
        if you go with exact search (read no ILIKE) then you can build an index and you're done. if you want fuzzy searching you're always doing a table scan, which isn't a problem now, but will be later.
      • a text index will scale much better.
      • lucifer
        cool, let's do the text index then? an exact search is not very useful.
      • ruaok
        agreed.
      • (on both points)
      • yvanzo
        It isn’t exposed via the MB WS.
      • lucifer
        TIL, Promotion Driven Development. https://twitter.com/GergelyOrosz/status/1442162...
      • ruaok
        yeah, I read something similar and it suggested the same -- it suggested that that is how we got the touchbar on MacBooks and many other "improvements".
      • lucifer
        oh. big corps are weird.
      • ruaok
        poor management rules will yield poor products.
      • lucifer
        +1
      • anyone has an example of a MB username with non ASCII characters?
      • ruaok
        reosarevok: ^^ ?
      • Etua has quit
      • lucifer
        nvm, i used a fake one.
      • ruaok: so i did some look up on full text search, it matches full word for user names that's not ideal. for partial search it supports only prefix matching, luc:* to match lucifer but no way to get an inword partial match or suffix match. alternatively, we can use `pg_trgm` extension to support better matches.
      • ruaok
        joy, another extension to install.
      • have you tried it?
      • lucifer
      • yup locally
      • ruaok
        works better?
      • lucifer
        yes works. full text refuses to match anything except prefixes or full word.
      • ruaok
        and installing the extension is just one command, among the others we already have?
      • that's great!
      • lucifer
        yup create extension `pg_trgm`;
      • reosarevok
        Sorry, I was away. I'm sure I could find some but I guess there's no need anymore? :)
      • ruaok
        reosarevok: nope. thanks.
      • lucifer: sounds like a good solution then, lets go for it.
      • riksucks
        ruaok lucifer
      • I took a look at your discussions. So based on that I think I should remove the pin part from the docstring, and a comment
      • also, ruaok, does `delete_user_timeline_event` seem like an okay name? Since `create_user_timeline_event` can only create notifications/recommendations, and the db method I wrote is technically the counter method of this method
      • that should be it, right?
      • lucifer
        ruaok: one catch: the index for pg_trgm is often larger than the table itself but this is a user table (small) so i guess we are covered?
      • ruaok
        seems fine, yes, lucifer
      • lucifer
        👍
      • ruaok
        riksucks: yep, sounds good.
      • monkey
        Happy to see some trial of PG-based fuzzy search at scale, I've been curious about that
      • lucifer
        if it works well enough, i'd vote to push it into a future BU PG module so that all *Brainz can use it.
      • opal has quit
      • opal joined the channel
      • Etua joined the channel
      • riksucks
        ruaok lucifer
      • I have committed according to the review. Do drop review whenever you guys are free.
      • yvanzo
        reosarevok: Is #2198 ready for review? or work in progress? or waiting for other PR?
      • reosarevok
        Well, it's mostly ready but I should probably make sure with bitmap on what small changes he wanted
      • Etua has quit
      • yvanzo: oh, I know https://github.com/metabrainz/musicbrainz-serve... is not in the milestone, but it's a 2-line bugfix, so if you have some time :)
      • BrainzGit
        [musicbrainz-server] 14reosarevok merged pull request #2286 (03master…MBS-11989): MBS-11989: Also correct rg to release_group for delete_alias https://github.com/metabrainz/musicbrainz-serve...
      • reosarevok
        yvanzo: what do you think, should I strip the favicon changes for now from the other URL tickets so we can release them to beta while I work on refactoring the favicon bits?
      • bitmap suggested ways to do that but they require thinking, so probably not happening today :D
      • yvanzo
        reosarevok: yes, if not happening today, set it as draft and rebase your other dependent PRs on master instead.
      • bitmap
        reosarevok: for https://github.com/metabrainz/musicbrainz-serve... I suggested removing all the redundant descriptions & I thought you agreed
      • yvanzo
        reosarevok, bitmap: maybe we should probably discuss the next milestone after beta freeze rather than waiting for prod release?
      • also moin
      • reosarevok
        bitmap: thanks for the reminder of what it was we decided, heh :)
      • "remove" meaning "give the description the same string as the label"?
      • bitmap
        moin!
      • well I suggested setting it to null, didn't we find out it's not used anywhere too?
      • reosarevok
        Well, in that case we should just have a second commit dropping them all, no?
      • yvanzo: can you double-check that we're not missing the point of them? :)
      • yvanzo
        It isn’t used at all?
      • bitmap
        that would be fine with me too
      • yvanzo
        I guess it was intended to be more explanatory just like description for relationship type. But in most cases, it isn’t needed, right?
      • bitmap
        right. they're mostly minor re-wordings like "Collections of type Wishlist" -> "Collections with type set to Wishlist"
      • reosarevok
        I guess unless we decide we want to show them somewhere with that description, we should just get rid of them all and make translators' lives easier
      • bitmap
        yvanzo: and yeah, I think that's the ideal point to create the next milestone
      • tandy[m]
        is this endpoint broken?
      • yvanzo
        Most of the current descriptions are useless, but some labels are misleading and should probably be replaced by the corresponding descriptions instead.
      • reosarevok
        yvanzo: wanna leave a comment with the ones you've noticed should be replaced?
      • I'll look into it once I'm done with these URL PRs
      • lucifer
        tandy[m]: no, we just don't generate them currently. that's changing soon. in a couple of weeks i hope, it should start serving up to date stats.
      • ruaok: did you get a chance to look at the charts on LB website?
      • reosarevok
        yvanzo: heh. So, for the vimeoondemand PR, if we use the current favicon code, it will work, but AFAICT only if I place them like this in constants.js:
      • Since it will catch the first if it matches, and not look any further
      • (unless I'm missing a good way to make the other vimeo string more specific)
      • Seems fine to me since it won't last long until we replace it with something better, hopefully, and still unbreaks adding those links, but just making sure you feel that's ok?
      • I guess I'll send it like that for now, note it on the commit msg
      • dseomn has quit
      • dseomn joined the channel
      • yvanzo
        reosarevok: yes, the vimeo PR is still missing some checks in the function "validate"
      • reosarevok
        Yeah, I'm on it
      • yvanzo: are you thinking of something like this?
      • Or do we have a better way I'm missing?
      • Also, I put a couple things on https://github.com/metabrainz/musicbrainz-serve... but I guess we should see what we can merge from the current one before we freeze and move the rest before we decide 100% on what to add
      • yvanzo
        reosarevok: Not a better way but what is commonly used in this file: switch/case. The URL format should be checked as well.
      • Yes, let's do the milestone after the freeze only, even tomorrow if needed.
      • reosarevok
        Ok
      • BrainzGit
        [musicbrainz-server] 14mwiencek opened pull request #2289 (03master…mbs-11983): MBS-11983: Try to fix some flaky Selenium release editor previews https://github.com/metabrainz/musicbrainz-serve...
      • reosarevok
        Yeah, I guess I can just do the case thing - I wasn't sure because this allows two options per entity type but I guess that's not actually an issue
      • yvanzo: but is there anything to check in the URL other than it not being a store link?
      • My understanding is this should work fine: