hi reosarevok, I blindly implemented MBS-10566 but it seems it might not be that useful: collections will need a schema change anyway, list of supported entity types for series could be defined in code constants rather than in table. Am I missing something?
(This change is not affecting replication slave servers, so we are pretty free to change our plans about it.)
reosarevok
yvanzo: why would collections need a schema change anyway?
If it's because we need editor_collection_whatever tables, can't we just create them already?
I expect having constraints in a table rather than hardcoded in code is for the same reason we have our country list for example in a table and not hardcoded in code :)
(we could *also* have the same constraint in code, to be doubly sure we don't try to do something that should be disallowed, but)
yvanzo
Yes, it's because we need editor_collection_foo tables. are we missing any?
These constraints for series exist only to prevent admins from adding a series type that would not be supported by the code. The only changes needed to support a new entity type in series are in code.
It cannot be compared with country list, it's not public, just internal implementation, referencing MB own entity types only.
That is thanks to the "login with github" feature I deployed ;)
reosarevok
outsidecontext: I get the feeling I asked this once already, but you don't happen to know whether libdiscid still causes these issues, do you? https://tickets.metabrainz.org/browse/MBS-1588
BrainzBot
MBS-1588: More user friendly page when trying to add a CD with a bad TOC
reosarevok is tempted to close it because "Bad Request / Sorry, there was a problem with your request. / Error message: The provided CD TOC is not valid" seems fine to me, although we could maybe add a bit more info about what can make a CD TOC invalid
yvanzo made a civil answer instead of blocking him right away.
outsidecontext
reosarevok: I don't even know whether there ever was such a bug in libdiscid
reosarevok
I believe nikki wouldn't have made it up, but :)
yvanzo: what do you think re: that ticket? Is the error message enough or should we try to improve it somehow?
yvanzo
reosarevok: how would you do that?
reosarevok
Maybe by adding some sort of "find out more" link to the error message linking to a doc that explains what makes a discid invalid
Either that, or trying to give different messages depending on exactly what invalidated the discid
yvanzo
Is there a way to detect that it was submitted using libdiscid (or using picard at least)?
reosarevok
I just dunno if it matters because it feels in most cases it's out of the abilities of the user to do something about it
Like, ok, they now know that it was invalid because it has the wrong number of tracks given or because the track offsets are not following a logical growing pattern
But does it matter to them? Can they fix it? :D
yvanzo
This request implies they have an account on MB website, right?
outsidecontext
reosarevok: I just mean this ticket does not give any information on how this disc ID was created, the other tickets it references are no longer available. It's hard to say what the bug is without any further details, and without the actual disc it might not be reproducible. And if the bug is unknown the question of if it was fixed can't be answered :)
reosarevok
outsidecontext: sure, understandable :) Just was wondering whether you had heard of any "invalid discids being created" bug
yvanzo: yes
yvanzo
Maybe just reword this error to say it’s not eidtor’s fault but more likely client’s fault and that bugs should be reported to either server or client if possible.
reosarevok
Well, not sure, actually, whether you can get here when adding a cdstub
So. "The provided CD TOC is not valid. This is probably an issue with the software you used to generate it. Try again and please report the error to your software maker if it persists" ?
yvanzo
Because the only help offered right now is to read the documentation, which isn’t really helpful but to read how to report a bug.
reosarevok: looks good to me :)
"report the error with the URL"
or "with the below technical information"
reosarevok
Sounds good
BrainzGit
[musicbrainz-server] yvanzo merged pull request #2050 (master…MBS-11524): MBS-11524: Disallow https://*.bandcamp.com/ URLs at release and recording level https://github.com/metabrainz/musicbrainz-serve...
reosarevok
yvanzo: do you have any idea what MBS-11579 is about? I don't get it
did you see the solution Mr_Monkey and I hacked out for finding gaps in timescale listens?
alastairp
ruaok: I saw the comment in irc from a few days back
ruaok
yea.
alastairp
haven't got as far as looking at email to see if there is a PR yet
ruaok
I can't believe the solution was sitting under my nose all this time.
no PR -- Mr_Monkey and I will tackle this together on wednesday. with chinese food, of course.
come join us
alastairp
I'll be there weds
ruaok
yay, great. I doubt we'll need help, but still. weds.
alastairp
so, we have these logical separations of the table, split into ~100 days or so. we decided when doing filtering that we didn't want to cross too many of these boundaries because otherwise the query took too long?
ruaok
the table is split into days currently, but yeah.
which gives us a perfect index as far as where we can find listens.
alastairp
and we had an issue that if a user skipped more than 1 logical boundary, we didn't know if it's because they had skipped one, or if they had no data before it?
ruaok
more or less year.
yeah
alastairp
but these aggregates let us collect summary information, which allows us at a glance (with a faster query) to see if a user has earlier/later data?
ruaok
so now when we want to fetch listens, we consult the listens_count table for the ranges and collect as many days as needed to fulfull the number of rows we were asked to fetch.
yes, exactly. in the end it may make this fetching query faster since we may be able to reduce the number of segments to search.
AND that graph of distribution of listens that monkey posted last week? easy to do!
alastairp
oh right, of course!
Mr_Monkey
That's the LastFM calendar view you were talking about alastairp
ruaok
I did mention that I love timescale, right?
alastairp
CatQuest: not sure if anyone replied to you. yes, if next/prev buttons dont' work as links then that's a bug and should be reported
Mr_Monkey
> in the end it may make this fetching query faster since we may be able to reduce the number of segments to search
And also avoiding repeated queries to the API from the front-end, that we currently do recursively if there's not enough listens to fill the page
shivam-kapila
ruaok: a good amount of times :)
alastairp
Mr_Monkey: hmm, why is the frontend doing multiple queries? in case it gets fewer than 25 listens in a single query?
ruaok
yerp
Mr_Monkey
Yes
alastairp
ah, wait
I was going to say "but how will we know that 1 more query back will get enough items?"
ruaok
there is a fair bit of logic of getting around the limitation. and now we can get rid of all that. #deletecodefuckyear
alastairp
but that's not an issue, because you have counts
ruaok
yep.
alastairp
so you just go back in time until sum(listens) is as many as you need
Mr_Monkey
Exactly
alastairp
does this also mean that we won't have to store counts in redis and they'll no longer get out of sync>
CatQuest
alastairp: oohhh I'll make a ticket of that then
alastairp
?
CatQuest
[11:10] <alastairp> CatQuest: not sure if anyone replied to you. yes, if next/prev buttons dont' work as links then that's a bug and should be reported
?
alastairp
CatQuest: yes, the ? was to be added to my previous line
CatQuest
ooohhh hah
(I was wodnering why you where cnfused only 5 minutes later)
alastairp
_lucifer: hi, I'm looking at the last items of the startup PR now