#metabrainz

MajorLurker: that is strange. I'll log on to my windows machine in a bit and see if I find anything

bitmap, hi, and thanks..... just for reference it was working fine till the end of last week (or thereabouts)

MajorLurker: np. that is suspicious timing wrt to the schema change but I can't imagine what it'd be off the top of my head, so we'll see

so EAC makes requests of the form /ws/2/discid/x?toc=y

and yet https://musicbrainz.org/ws/2/discid/9kjVvtD69p3... works fine

we did deploy https://github.com/metabrainz/musicbrainz-serve... during the schema change, but this applies equally to old and new releases

MajorLurker: just to check, you're inserting the cd and then clicking the MusicBrainz icon ("Get CD Information From Metadata Provider"), right? does it show some kind of error?

bitmap, haven't changed anything I do but if I hit "Get CD Information from etc" it looks but does not find in the case of the first link I provided, or a FreeDB link for the second

no errors are indicated in each case

FYI Picard finds the disc just fine as well

MajorLurker: I'm a bit stumped, especially if picard finds it. :\ maybe a different metadata provider got selected somehow?

if I go (in the menu) to EAC -> Metadata Options... I have "MusicBrainz Metadata Plugin" selected there. EAC v1.6

bitmap, thing is it findds the "old" entry CD's ok.... it's just the new ones that seem to be the issue

yeah

though I checked our server logs to see what queries EAC was making for those new CDs, and curiously, didn't find any

so it seems like it's not trying to use MusicBrainz at all

Ok well having just seen what you posted above about "MusicBrainz Metadata Plugin" I looked and I did not have that selected. I selected it and it's working fine now....

Who knows why some were working and not others.... oh well problem solved... thanks for the help

hah! I wasn't sure if it'd be that simple. glad it's working

I did update EAC but that was yesterday, I had the problem before that.... anyway one of those mysteries in life...

yvanzo: While the UI of external links editor still needs discussion, I took some time to look into the week 2 task

bitmap: Thanks for your comment about the PR. Yes, the relationship editor should be used in the end. It's just an attempt to make incremental changes.

At least, some issues have been spotted already that can be worked on :)

zas: ruaok: alastairp: some particular api requests are causing a 504 on LB. does this mean the server is taking too long to generate a response in that particular case?

yvanzo: The current bubbles are implemented with template toolkit and jQuery UI, is it OK that I implement a new component using React?

https://api.listenbrainz.org/1/stats/user/accra... consistently gives 504s

https://api.listenbrainz.org/1/stats/user/amcap... works fine

bitmap: Thanks for your constructive suggestions

yyoung: I would suggest to look into the errors issue at first, as it will be needed at some point, whatever the lib being used for display.

Alright.

URLCleanup currently returns an object {error: 'string', result: boolean}; Maybe a third attribute can be added to specify error's target(s).

Some other errors are directly thrown from externalLinks.js.

Yes

yvanzo: But for some errors like "this link should not be this type", what target will it be, link or type?

type

And errors like "this link should be added to another entity" targets the link?

right

So maybe the message "This URL is not allowed for the selected link type, or is incorrectly formatted." can also be splitted into 2 types of error.

yyoung: link to this?

"This URL is not allowed for the selected link type" targets type, and "This URL is incorrectly formatted." targets link, I think?

But we need to check if URLCleanup can distinguish between these two.

Can you link to the code please? :)

yvanzo: I'd expect validation errors would always go to the type, since the validation is type-specific

Unless we change that later :)

While something like

Should probably go on the url

yvanzo: https://github.com/metabrainz/musicbrainz-serve...

But

Should probably also go on the type, so the errors inside ExternalLinksEditor probably do need specifying )

reosarevok: it's not, sometimes it's related to the entity type.

I mean, sure, but it's still for the specific type, not the URL field, no? Since the entity type can't be changed

Like, if you had several types selected for the same URL, it would only apply to one (or more) of them

You might have a case where the same link is allowed for type a and blocked for type b :)

So it wouldn't be an issue with the URL but the URL for this specific type - unless "link is required" or "this is a link to MusicBrainz"

reosarevok: if there is an URL that won't be accepted for that entity because it cannot be linked to this entity type, shouldn't we display the error below the URL input field and not let relationship type being selected?

IMO no, we should either not show an error at all and just not have the relationship type displayed at all for choosing, or display the error under the field when chosen,

These URLs cannot be linked with any other relationship type if I'm not missing anything else.

and since we're making errors more helpful than "nope you can't", it makes sense to still show it

Sure, that's fair that right now validation implies autoselect so there's only one valid type

But we're changing that, right? :)

So eventually it might be invalid for one of the autoselect options, but not for others

Another thing yyoung could work on is to actually make this code already support multiple types in one entry rather than pasting multiple times, I guess :)

(add some "add more types" button or whatever)

Especially if you feel that's not an obvious feature (which it might not be, most repeated entries right now are probably bandcamp links entered by the importer rather than something added by hand by an editor)

reosarevok: we are changing that and we are not changing the fact that some URL patterns won't be accepted to be linked with some entity types.

Yes, but :)

lucifer: clearly due to a timeout, but where... does it take longer than 1 minute to answer?

Let's say you allow bandcamp links to autoselect for streaming, and for free download

hah, ok, I'll answer this and then shut up a bit :D

reosarevok: Yeah that's a future task, but bitmap has some thoughts on it

For some entity type, it might be that only streaming is allowed, but not download, with a specific pattern

lucifer: uwsgi_read_timeout defaults to 60s, and it seems this endpoint proxy is using default. But it can also be an issue on backend.

We should still allow both options to be selected, just so we can show a helpful error when the user tries to select download, like "hey, this link can't be linked as download for reason X, maybe you want link Y instead"

(the alternative, just showing only streaming if that's the only allowed match for the pattern, would probably confuse the user unless it's very obvious why download is blocked)

reosarevok: Are we going to hide some link type options for some entities?

reosarevok, yyoung: What are the possible targets for errors: URL, each of selected types, all types, both URL and types. Would that help with covering this case?

reosarevok: right, the relationship type should not be displayed at all, but that can be improved later on.

lucifer: I don't have any info in logs about after how much time it timeouts, but according to config it should be 60s. Do you have any error on backend that occurs before 1 minute delay?

zas: no, i couldn't find anything useful in sentry. will go through container logs next.

yyoung: I'd suggest hiding from the dropdown (for urls that do not get autoselected) all options that should only work with autoselect

reosarevok: the same idea as for the current PR: making incremental changes even though it is not optimized. but it seems a lot of redesign discussion is still needed before that.

So, if the user pastes an official website or something, all the relationship types like Wikidata, Discogs, etc

Won't ever be acceptable with the link if it didn't autoselect. so they are just noise :)

zas: however, i did execute the db query which the 504ing api calls and it was instantaneous.

i'll look further and let you know if somthing comes up.

In the vast majority of cases, we should end up with a compact UI, just like the current one, plus buttons for dialog boxes to handle the details.

(I'm waiting for something like that to happen before I split "other databases" into one relationship type per database - right now that'd uselessly make the list too long for users in the by-hand use case)

yvanzo: sure, I actually like the type being underneath, personally

"split other databases"?

Yes, add separate relationship types for each library and website, rather than lumping them into "other databases" :)

??

"other databases" describes a role, I'm not sure why you want to split it.

The target website is already known from the URL itself.

https://musicbrainz.org/doc/Other_Databases_Rel... - for data usage, it would be a lot simpler if each of these had a specific relationship type, so you can just find stuff using the "Rate Your Music" relationship without having to check every otherdbs URL

will you split "streaming" over all the possible streaming websites too?

Yes :)

Well, all the ones that we autoselect

has this plan been discussed somewhere? I did miss that.

It's been something I meant to do for four years or so :) So I've discussed it on IRC a lot, the forums too, not sure if there's a ticket, I'll see

lucifer: check backend logs, if you feel we need to increase timeout delay for this specific app, just tell me, but we need to be sure it is strictly needed (behavior is normal, it just needs more time to answer). If db query is fast, I suspect a bug on backend preventing it to answer in time.

reosarevok: That would require extra rules, I guess

Seems not. In any case, if you check https://musicbrainz.org/relationships/artist-url then all the stuff under otherdatabases is not significantly different that the stuff autoselected *into* other databases

reosarevok: it would also make more difficult to find links to other databases of any type.

It just got grandfathered in :)

Well, it already is, since Discogs, allmusic, etc have their own separate types already

So it's not like we are consistent

I always considered the website-specific relationship types as temporary workaround.

If that was the case, we would have merged them already, since nothing blocks it :)

haha

Sorry :)

reosarevok: we would have merged them only once the code allowed to handle website specifics.

yyoung: splitting the thing (or lumping the ones currently split) wouldn't require a lot of extra rules, most of the other databases already validate separately

yvanzo: but we already have separate validation and cleanup for each one now, so nothing is missing, is it? :)

what is missing: allowing to search for "streaming" relationships, YT and all included.

yyoung: hiding all the autoselect-only rels might actually require new extra rules - it might even require a flag "this should be autoselect" on the relationship type (schema change) so the code doesn't have to run all the validation before figuring out what to show

Don't worry about that too much for now :)

similarly for Amazon links: having "purchase as ..." links instead

yvanzo: I agree with Amazon because it's useful to separate "purchase for download" links and "purchase for mail order" links :)

reosarevok: But we need to iterate all options to find possible ones, don't we?

yyoung: yes, that's why I mentioned possibly adding a flag

Amazon rel type cannot just be a child of any other relationship type as it can be either streaming, download, and/or mail-order.

Yeah. We might eventually want to split Amazon into all of those, just like we do with bandcamp

iTunes/Apple is similar mess.

Once we support multiple options

I'm always thinking if we can transform it into a rules set of regexps, etc

yyoung: isn't it already basically that? :)

Basically, for purchase links, I agree we shouldn't have separate per-store relationships because of the mail order vs download vs free download vs streaming issue

if this is just about search, we can have add a specific searchable field in search indexes, just like we have views rather than tables for series.