bitmap doesn't have many (any?) unsubmitted cds to test with
2021-05-26 14614, 2021
bitmap
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?
2021-05-26 14607, 2021
MajorLurker
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
2021-05-26 14619, 2021
MajorLurker
no errors are indicated in each case
2021-05-26 14643, 2021
MajorLurker
FYI Picard finds the disc just fine as well
2021-05-26 14609, 2021
bitmap
MajorLurker: I'm a bit stumped, especially if picard finds it. :\ maybe a different metadata provider got selected somehow?
2021-05-26 14656, 2021
bitmap
if I go (in the menu) to EAC -> Metadata Options... I have "MusicBrainz Metadata Plugin" selected there. EAC v1.6
2021-05-26 14615, 2021
thomasross has quit
2021-05-26 14633, 2021
MajorLurker
bitmap, thing is it findds the "old" entry CD's ok.... it's just the new ones that seem to be the issue
2021-05-26 14617, 2021
bitmap
yeah
2021-05-26 14643, 2021
bitmap
though I checked our server logs to see what queries EAC was making for those new CDs, and curiously, didn't find any
2021-05-26 14605, 2021
bitmap
so it seems like it's not trying to use MusicBrainz at all
2021-05-26 14623, 2021
MajorLurker
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....
2021-05-26 14643, 2021
MajorLurker
Who knows why some were working and not others.... oh well problem solved... thanks for the help
2021-05-26 14600, 2021
bitmap
hah! I wasn't sure if it'd be that simple. glad it's working
2021-05-26 14618, 2021
bitmap is thankful it's not a bug he has to fix :)
2021-05-26 14646, 2021
MajorLurker
I did update EAC but that was yesterday, I had the problem before that.... anyway one of those mysteries in life...
2021-05-26 14610, 2021
tykling_ is now known as tykling
2021-05-26 14607, 2021
captainepoch joined the channel
2021-05-26 14639, 2021
the4oo4 joined the channel
2021-05-26 14648, 2021
captainepoch has quit
2021-05-26 14604, 2021
captainepoch joined the channel
2021-05-26 14622, 2021
agatzk has quit
2021-05-26 14642, 2021
agatzk joined the channel
2021-05-26 14614, 2021
yyoung
yvanzo: While the UI of external links editor still needs discussion, I took some time to look into the week 2 task
2021-05-26 14612, 2021
yvanzo
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.
2021-05-26 14630, 2021
yvanzo
At least, some issues have been spotted already that can be worked on :)
2021-05-26 14620, 2021
lucifer
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?
2021-05-26 14631, 2021
yyoung
yvanzo: The current bubbles are implemented with template toolkit and jQuery UI, is it OK that I implement a new component using React?
Should probably also go on the type, so the errors inside ExternalLinksEditor probably do need specifying )
2021-05-26 14604, 2021
yvanzo
reosarevok: it's not, sometimes it's related to the entity type.
2021-05-26 14641, 2021
reosarevok
I mean, sure, but it's still for the specific type, not the URL field, no? Since the entity type can't be changed
2021-05-26 14658, 2021
reosarevok
Like, if you had several types selected for the same URL, it would only apply to one (or more) of them
2021-05-26 14621, 2021
reosarevok
You might have a case where the same link is allowed for type a and blocked for type b :)
2021-05-26 14632, 2021
reosarevok
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"
2021-05-26 14645, 2021
yvanzo
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?
2021-05-26 14631, 2021
reosarevok
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,
2021-05-26 14639, 2021
yvanzo
These URLs cannot be linked with any other relationship type if I'm not missing anything else.
2021-05-26 14650, 2021
reosarevok
and since we're making errors more helpful than "nope you can't", it makes sense to still show it
2021-05-26 14637, 2021
reosarevok
Sure, that's fair that right now validation implies autoselect so there's only one valid type
2021-05-26 14640, 2021
reosarevok
But we're changing that, right? :)
2021-05-26 14655, 2021
reosarevok
So eventually it might be invalid for one of the autoselect options, but not for others
2021-05-26 14622, 2021
reosarevok
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 :)
2021-05-26 14631, 2021
reosarevok
(add some "add more types" button or whatever)
2021-05-26 14654, 2021
reosarevok
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)
2021-05-26 14622, 2021
yvanzo
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.
2021-05-26 14650, 2021
reosarevok
Yes, but :)
2021-05-26 14647, 2021
zas
lucifer: clearly due to a timeout, but where... does it take longer than 1 minute to answer?
2021-05-26 14600, 2021
reosarevok
Let's say you allow bandcamp links to autoselect for streaming, and for free download
2021-05-26 14601, 2021
yvanzo is still trying to read all of your comments in the backlog...
2021-05-26 14617, 2021
reosarevok
hah, ok, I'll answer this and then shut up a bit :D
2021-05-26 14641, 2021
yyoung
reosarevok: Yeah that's a future task, but bitmap has some thoughts on it
2021-05-26 14643, 2021
reosarevok
For some entity type, it might be that only streaming is allowed, but not download, with a specific pattern
2021-05-26 14620, 2021
zas
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.
2021-05-26 14622, 2021
reosarevok
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"
2021-05-26 14656, 2021
reosarevok
(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)
2021-05-26 14602, 2021
reosarevok shuts up, let's yvanzo read :)
2021-05-26 14635, 2021
yyoung
reosarevok: Are we going to hide some link type options for some entities?
2021-05-26 14647, 2021
yvanzo
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?
2021-05-26 14609, 2021
yvanzo
reosarevok: right, the relationship type should not be displayed at all, but that can be improved later on.
2021-05-26 14610, 2021
zas
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?
2021-05-26 14647, 2021
lucifer
zas: no, i couldn't find anything useful in sentry. will go through container logs next.
2021-05-26 14652, 2021
reosarevok
yyoung: I'd suggest hiding from the dropdown (for urls that do not get autoselected) all options that should only work with autoselect
2021-05-26 14617, 2021
yvanzo
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.
2021-05-26 14619, 2021
reosarevok
So, if the user pastes an official website or something, all the relationship types like Wikidata, Discogs, etc
2021-05-26 14639, 2021
reosarevok
Won't ever be acceptable with the link if it didn't autoselect. so they are just noise :)
2021-05-26 14655, 2021
lucifer
zas: however, i did execute the db query which the 504ing api calls and it was instantaneous.
2021-05-26 14621, 2021
lucifer
i'll look further and let you know if somthing comes up.
2021-05-26 14623, 2021
yvanzo
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.
2021-05-26 14641, 2021
reosarevok
(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)
2021-05-26 14659, 2021
reosarevok
yvanzo: sure, I actually like the type being underneath, personally
2021-05-26 14659, 2021
yvanzo
"split other databases"?
2021-05-26 14619, 2021
reosarevok
Yes, add separate relationship types for each library and website, rather than lumping them into "other databases" :)
2021-05-26 14626, 2021
yvanzo
??
2021-05-26 14659, 2021
yvanzo
"other databases" describes a role, I'm not sure why you want to split it.
2021-05-26 14619, 2021
yvanzo
The target website is already known from the URL itself.
2021-05-26 14639, 2021
reosarevok
https://musicbrainz.org/doc/Other_Databases_Relat… - 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
2021-05-26 14640, 2021
yvanzo
will you split "streaming" over all the possible streaming websites too?
2021-05-26 14644, 2021
reosarevok
Yes :)
2021-05-26 14651, 2021
reosarevok
Well, all the ones that we autoselect
2021-05-26 14615, 2021
yvanzo
has this plan been discussed somewhere? I did miss that.
2021-05-26 14641, 2021
reosarevok
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
2021-05-26 14600, 2021
zas
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.
2021-05-26 14619, 2021
yyoung
reosarevok: That would require extra rules, I guess
2021-05-26 14654, 2021
reosarevok
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
2021-05-26 14656, 2021
yvanzo
reosarevok: it would also make more difficult to find links to other databases of any type.
2021-05-26 14658, 2021
reosarevok
It just got grandfathered in :)
2021-05-26 14622, 2021
reosarevok
Well, it already is, since Discogs, allmusic, etc have their own separate types already
2021-05-26 14629, 2021
reosarevok
So it's not like we are consistent
2021-05-26 14631, 2021
yvanzo
I always considered the website-specific relationship types as temporary workaround.
2021-05-26 14649, 2021
reosarevok
If that was the case, we would have merged them already, since nothing blocks it :)
2021-05-26 14658, 2021
yyoung is shocked by the complexity and possibility of this module, again
2021-05-26 14602, 2021
reosarevok
haha
2021-05-26 14606, 2021
reosarevok
Sorry :)
2021-05-26 14635, 2021
yvanzo
reosarevok: we would have merged them only once the code allowed to handle website specifics.
2021-05-26 14642, 2021
reosarevok
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
2021-05-26 14603, 2021
reosarevok
yvanzo: but we already have separate validation and cleanup for each one now, so nothing is missing, is it? :)
2021-05-26 14645, 2021
yvanzo
what is missing: allowing to search for "streaming" relationships, YT and all included.
2021-05-26 14601, 2021
reosarevok
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
2021-05-26 14619, 2021
reosarevok
Don't worry about that too much for now :)
2021-05-26 14623, 2021
yvanzo
similarly for Amazon links: having "purchase as ..." links instead
2021-05-26 14603, 2021
reosarevok
yvanzo: I agree with Amazon because it's useful to separate "purchase for download" links and "purchase for mail order" links :)
2021-05-26 14605, 2021
yyoung
reosarevok: But we need to iterate all options to find possible ones, don't we?
2021-05-26 14617, 2021
reosarevok
yyoung: yes, that's why I mentioned possibly adding a flag
2021-05-26 14625, 2021
yvanzo
Amazon rel type cannot just be a child of any other relationship type as it can be either streaming, download, and/or mail-order.
2021-05-26 14646, 2021
reosarevok
Yeah. We might eventually want to split Amazon into all of those, just like we do with bandcamp
2021-05-26 14657, 2021
yvanzo
iTunes/Apple is similar mess.
2021-05-26 14659, 2021
reosarevok
Once we support multiple options
2021-05-26 14601, 2021
reosarevok
Yeah
2021-05-26 14619, 2021
yyoung
I'm always thinking if we can transform it into a rules set of regexps, etc
2021-05-26 14633, 2021
reosarevok
yyoung: isn't it already basically that? :)
2021-05-26 14657, 2021
reosarevok
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
2021-05-26 14657, 2021
yvanzo
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.
