#metabrainz

It doesn't necessarily require to change the data structure into the db.

It doesn't, but other databases was always a hack because we couldn't do better :)

reosarevok: Yes, but after supporting multiple rels we may need to validate rels combinations, that might be complicated to do in each function

yvanzo: having separate rels allows for having separate guidelines, too :)

yyoung: I'm expecting for now that we'd still validate like we do now, each url once for each option in the multiple rels supported

Do we have a use case for "block X and Y in combination, but allow both separately"?

reosarevok: That's what I want to know :)

I can't think of any rn but maybe yvanzo can

Or combination A and B is valid but A and C is not

Or a type of link can only have one relationship

yyoung: probably but before over-engineering anything, you would want to have an overview of real use cases

have a look at current edits and community forums, that might help you with :)

The closest I can think of is "we might not want to allow (or at least give a warning for) adding more than one link of the same type"

So, "this already has a discogs link, are you sure you want to add another discogs link? there should generally be only one"

But that should be easier than what you were mentioning :)

for example bandcamp can offer streaming, download, and mail-order

(and also not a requirement)

reosarevok: But URLCleanup knows nothing about other links, so...

Yeah, sure, that shouldn't probably be part of your work rn :)

Just thinking of combinations that might be a clash more generally because of the topic

And that should also probably be a flag on the relationship type, then looked at by externalLinks, if anything :) No worries rn

Hopefully :)

yvanzo, reosarevok: So where are we now, what about MBS-11680?

MBS-11680: New UI for external links editor to display relationships as a list https://tickets.metabrainz.org/browse/MBS-11680

Yeah. We can even ask in the forums for a list of URLs that can be of multiple types

So that we have a general idea which ones are there :)

re: yvanzo's comment earlier

yyoung: I'd suggest if we want to make this change and merge it, then it should actually support using the list :)

So, for non-auto-selected things, allow adding and selecting more types

the list?

Well, the idea is to "display relationships as a list."

But right now it's not a list, it's just one underneath, right?

that is related to using the relationship editor, right?

You still can't select more than one type

(for that you can only add a new link)

(well, add the same link again)

reosarevok: does it seem reasonnable to add support for errors target already?

yvanzo: IMO yes, if it's not too hard. But I'd still prioritize adding multiple reltypes to one link + blocking adding the same link twice.

so that we can postpone the UI stuff to have bitmap's input about using relationship editor instead of reinventing the wheel.

Oh, ok, in that sense

Sure, why not

Also bitmap raises a concern that the new UI takes more space

yes and he is correct but that was not intended to be the final UI

It does, yes. One alternative to that to take less space would be to still have the type as the label if there's only one, but move it underneath if there's more than one. But that might be confusing

that would be confusing indeed

yvanzo: How can we solve this problem with relationship editor?

yyoung: let's wait for having bitmap in the loop for this topic, it is the middle of his night right now.

Mind, I'm ok with it taking a bit more space :) I didn't think it was a big issue, although I guess it might be more so for very large entities with a ton of URLs

Yeah, let's get some more bitmap feedback

yvanzo: Yes, but he suggests posting a thread on the forum to get more feedbacks

That also wouldn't hurt, especially if we throw the code up on test.mb

So that people can actually see it

reosarevok: the main point of this PR was to group rels by URL, not to optimize UI in any other way. For example, if errors are better handled, we can hide rel type selector if error is about the URL itself. There are a lot of possible optimizations.

yyoung: We seem to all agree that errors need better handling, that is a prerequisite improvement that can be done without changing the UI.

yvanzo: I get that :) It doesn't yet do that though, right? Since it still has only one rel per URL

yyoung: Also documenting the most popular use cases about relationship types combinations would be nice.

reosarevok: yes, it doesn't, just rearranged layout to allow that later on. I didn't see it would cause a regression.

Ok :)

Before the UI change this disadvantage of error handling was not that obviously, though

I personally don't mind the layout change (other than the label index which is still weird to me) but before merging the change into beta/prod I'd want it to actually support multiple link types, since otherwise the community might be confused about the reasons :)

We could always have a branch we merge PRs against though, and then merge that into master when something is feature completeish?

yyoung: yes, your PR helped with finding more issues we didn't anticipate earlier :)

As it always happens :D

Please don't be discouraged :)

I switched it to draft status so that is more clear we are not willing to merge it as-is.

The dropdown list is so irritating to use. Just restricting the entries to those detected as "permitted" would be a big usability gain IMV.

Please ping me when you have something on beta, I use the URL links a lot so would be interested in the changes and happy to provide feedback.

We might have it on test before beta :) But we'll let people know if we do

Hi, _lucifer: i had a question about CB. in the reviews table the 'source' table either contains 'BBC' if the review was imported, or it is null otherwise correct?

yvanzo: OK, so is there anything else I need to do on this PR?

jasondk: let me check for you

thanks!

atj: Agreed, and that would be great with your help :)

I do think it will be really difficult to get right by the way - it's a complex problem to map

My view would be that you make the 95% of use cases easier and try not to make the other 5% harder :)

jasondk: yep, only null or 'BBC'

And if you make 95% a lot easier, then you can make the 5% a tiny bit harder, just not a lot :D

agreed

yyoung: Not for now I guess. You might want to look into relationship editor code to see how to possibly hook URLCleanup to it.

Is there a report on the frequency of different link types?

No, there are just statistics on relationship types: https://musicbrainz.org/statistics/relationships

No->Yes

But since, say, otherdatabases are lumped, then it's not too useful for that - plus there's no way to see "how many bandcamp links are free streaming vs free download vs purchase"

Still a lot better than nothing tho

I think we have a general idea where we stand, right?

Great. for my gsoc project i was planning on creating a new enum type to alter that column to accept 'BBC' or 'listenbrainz' or null as valid values

I did such statistics by host name in the past, I will try to find it back.

jasondk: the BBC reviews are a special case, you can just ignore this column and always set it to null

yvanzo: Sure. But the next task on the schedule is a bubble to display cleaned-up URL, I think that wouldn't introduce other issues?

mmm, although - now that I think about it, that's an interesting idea. so the idea is that we'd be able to identify which reviews were written on critiquebrainz and which ones were written on listenbrainz?

is that page showing that only 10% of artists have 1 or more URLs?

yyoung: this might change with redesign discussion going on :)

is the source of the review the interesting data, or is the type of review the interesting data (e.g. long or short review)

yvanzo: Alright :)

alastairp: Yes, and then it would be indicated on the site in the future because I imagine reviews coming in from LB and CB would look different

So i was just thinking to use the source column already existing

let me read your proposal again

atj: good question, I'm not sure what 100% means for “has a discography page at” either

jasondk: hmm, I think a new `review_type` column with values short/long would make a bit more sense. It's always a good idea to be explicit rather than try to reuse an existing feature

now that I remember reviews coming from LB wont have a character cap but iliekcomputers mentioned that it would be a good idea to distinguish between lb and cb

right. I would come up with a decision on what the difference is between the reviews. If it's short/long then we should use that as the distinguishing feature. If the only difference is the website that it comes from then maybe we don't need to track that

alastairp: That makes sense. we would have to decide on how many characters a short vs long review is

atj: IIRC that means "of all relationships, 10% are artist-url ones

Yeah, and then we could look through the existing reviews and sort them.

yvanzo: a bug, that's what that means - it seems to be the stats for "discography" above

I'll try to look into it after breakfast

ah yes, I see it's 10% of all relationships

was there a page/tag for artists with MB accounts?

Just had the artist correct one of my submissions :)

IIRC chaban had a collection or something

lucifer: hi, I started https://github.com/metabrainz/python-consul-sta...

alastairp: thanks! i'll read it carefully. could you please also add a section on how git2consul fits in here?

my understanding is that it takes the json file from docker-server-configs directory and submits those to consul agent. but i am not sure if that is correct and if it is how that works or any gotchas to keep in mind.

I'm not sure how much detail we should go into with this repo, if it should be completely self-contained or not

or if we should focus on the documentation instead

yes, I wondered about that. keep in mind that I'm trying to document the app side of things, not the config/server side of things

i think its fine to keep the repo for only stuff that we have to handle in a project itself. remaining parts of the setup can live somewhere else and maybe just add a link here.

right makes sense.

but also - we have this ongoing plan to replace some of the consul server parts, so we shouldn't go into that in too much more detail

oh ok, sure that can be left for now then.

lucifer: btw, did dumps work last night? (with the dumps directory not working)

alastairp: nope no dump last night, also storage box was still down when i checked this morning.

did we try and just unmount/remount it?

sure, let's try that

umount /mnt/dumps and then mount.cifs from history right?

yes

seems to be working now

ls /mnt/dumps displays contents now

i'll trigger an incremental dump then

maybe restart cron too

just to be sure it has the right mount

makes sense, doing that first

dump is now up on ftp.

ruaok: alastairp: the 504s I mentioned earlier are related to labs api, the artist map endpoint calls labs api internally. labs api is 504ing on some requests, we probably do not handle that correctly so the artist map endpoint also returns a 504 to frontend.

yvanzo: oh no, figured out the problem

https://musicbrainz.org/relationships/artist-url - both the parent grouping rel and the specific rel are called "discography"

so the stats are overwriting each other

Meaning all our historical data is bogus :/

I'll rename the children rel to discography page

I opened (and quickly closed) MBS-11683 for the issue

MBS-11683: "has a discography page at" stats are broken https://tickets.metabrainz.org/browse/MBS-11683