#metabrainz

Is BrainzBot down?

mo’’in’

yes

(BrainzGit)

moin!

coffee, yvanzo ?

no thanks :)

Hey ruaok , if you are free, would you review my PR?

will do today, riksucks!

thanks ruaok :D

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.

hi. Hmm, ok, but where are you expecting the artist_mbids column to be inserted? can you paste me the order you're expecting?

this is the current order, https://github.com/metabrainz/listenbrainz-serv...

oh, right we had them before. sorry. ok, will fix.

np :)

were you seeing this order?

yes

this is the change i made to spark to test these dumps - https://github.com/metabrainz/listenbrainz-serv...

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?

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

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?

never used a text search index before so not sure. but no harm in trying it :D

I would approve of that approach, but ILIKE will blow up in our faces in the future.

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.

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.

yeah :/. to be sure, you mean ILIKE and the text search index both have scalability issues or only the former?

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.

agreed.

(on both points)

It isn’t exposed via the MB WS.

TIL, Promotion Driven Development. https://twitter.com/GergelyOrosz/status/1442162...

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".

oh. big corps are weird.

poor management rules will yield poor products.

+1

anyone has an example of a MB username with non ASCII characters?

reosarevok: ^^ ?

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.

joy, another extension to install.

have you tried it?

yup locally

works better?

yes works. full text refuses to match anything except prefixes or full word.

and installing the extension is just one command, among the others we already have?

that's great!

yup create extension `pg_trgm`;

Sorry, I was away. I'm sure I could find some but I guess there's no need anymore? :)

reosarevok: nope. thanks.

lucifer: sounds like a good solution then, lets go for it.

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?

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?

seems fine, yes, lucifer

👍

riksucks: yep, sounds good.

Happy to see some trial of PG-based fuzzy search at scale, I've been curious about that

if it works well enough, i'd vote to push it into a future BU PG module so that all *Brainz can use it.

ruaok lucifer

I have committed according to the review. Do drop review whenever you guys are free.

reosarevok: Is #2198 ready for review? or work in progress? or waiting for other PR?

Well, it's mostly ready but I should probably make sure with bitmap on what small changes he wanted

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 :)

[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...

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

reosarevok: yes, if not happening today, set it as draft and rebase your other dependent PRs on master instead.

reosarevok: for https://github.com/metabrainz/musicbrainz-serve... I suggested removing all the redundant descriptions & I thought you agreed

reosarevok, bitmap: maybe we should probably discuss the next milestone after beta freeze rather than waiting for prod release?

also moin

bitmap: thanks for the reminder of what it was we decided, heh :)

"remove" meaning "give the description the same string as the label"?

moin!

well I suggested setting it to null, didn't we find out it's not used anywhere too?

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? :)

It isn’t used at all?

that would be fine with me too

I guess it was intended to be more explanatory just like description for relationship type. But in most cases, it isn’t needed, right?

right. they're mostly minor re-wordings like "Collections of type Wishlist" -> "Collections with type set to Wishlist"

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

yvanzo: and yeah, I think that's the ideal point to create the next milestone

is this endpoint broken?

Most of the current descriptions are useless, but some labels are misleading and should probably be replaced by the corresponding descriptions instead.

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

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?

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

reosarevok: yes, the vimeo PR is still missing some checks in the function "validate"

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

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.

Ok

[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...

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: