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.)
2021-04-12 10242, 2021
reosarevok
yvanzo: why would collections need a schema change anyway?
2021-04-12 10203, 2021
reosarevok
If it's because we need editor_collection_whatever tables, can't we just create them already?
2021-04-12 10248, 2021
reosarevok
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 :)
2021-04-12 10212, 2021
reosarevok
(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)
2021-04-12 10247, 2021
yvanzo
Yes, it's because we need editor_collection_foo tables. are we missing any?
2021-04-12 10209, 2021
yvanzo
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.
2021-04-12 10249, 2021
yvanzo
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 ;)
2021-04-12 10229, 2021
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
2021-04-12 10230, 2021
BrainzBot
MBS-1588: More user friendly page when trying to add a CD with a bad TOC
2021-04-12 10219, 2021
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
2021-04-12 10215, 2021
yvanzo made a civil answer instead of blocking him right away.
2021-04-12 10245, 2021
outsidecontext
reosarevok: I don't even know whether there ever was such a bug in libdiscid
2021-04-12 10255, 2021
reosarevok
I believe nikki wouldn't have made it up, but :)
2021-04-12 10213, 2021
reosarevok
yvanzo: what do you think re: that ticket? Is the error message enough or should we try to improve it somehow?
2021-04-12 10244, 2021
yvanzo
reosarevok: how would you do that?
2021-04-12 10223, 2021
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
2021-04-12 10235, 2021
reosarevok
Either that, or trying to give different messages depending on exactly what invalidated the discid
2021-04-12 10247, 2021
yvanzo
Is there a way to detect that it was submitted using libdiscid (or using picard at least)?
2021-04-12 10250, 2021
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
2021-04-12 10240, 2021
reosarevok
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
2021-04-12 10247, 2021
reosarevok
But does it matter to them? Can they fix it? :D
2021-04-12 10200, 2021
yvanzo
This request implies they have an account on MB website, right?
2021-04-12 10218, 2021
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 :)
2021-04-12 10205, 2021
reosarevok
outsidecontext: sure, understandable :) Just was wondering whether you had heard of any "invalid discids being created" bug
2021-04-12 10208, 2021
reosarevok
yvanzo: yes
2021-04-12 10232, 2021
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.
2021-04-12 10232, 2021
reosarevok
Well, not sure, actually, whether you can get here when adding a cdstub
2021-04-12 10250, 2021
reosarevok
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" ?
2021-04-12 10257, 2021
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.
2021-04-12 10200, 2021
yvanzo
reosarevok: looks good to me :)
2021-04-12 10246, 2021
yvanzo
"report the error with the URL"
2021-04-12 10241, 2021
yvanzo
or "with the below technical information"
2021-04-12 10214, 2021
reosarevok
Sounds good
2021-04-12 10250, 2021
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-server/…
2021-04-12 10243, 2021
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?
2021-04-12 10209, 2021
alastairp
ruaok: I saw the comment in irc from a few days back
2021-04-12 10217, 2021
ruaok
yea.
2021-04-12 10221, 2021
alastairp
haven't got as far as looking at email to see if there is a PR yet
2021-04-12 10226, 2021
ruaok
I can't believe the solution was sitting under my nose all this time.
2021-04-12 10246, 2021
ruaok
no PR -- Mr_Monkey and I will tackle this together on wednesday. with chinese food, of course.
2021-04-12 10252, 2021
ruaok
come join us
2021-04-12 10253, 2021
alastairp
I'll be there weds
2021-04-12 10208, 2021
ruaok
yay, great. I doubt we'll need help, but still. weds.
2021-04-12 10223, 2021
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?
2021-04-12 10256, 2021
ruaok
the table is split into days currently, but yeah.
2021-04-12 10207, 2021
ruaok
which gives us a perfect index as far as where we can find listens.
2021-04-12 10211, 2021
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?
2021-04-12 10233, 2021
ruaok
more or less year.
2021-04-12 10236, 2021
ruaok
yeah
2021-04-12 10208, 2021
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?
2021-04-12 10218, 2021
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.
2021-04-12 10251, 2021
ruaok
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.
2021-04-12 10221, 2021
ruaok
AND that graph of distribution of listens that monkey posted last week? easy to do!
2021-04-12 10243, 2021
alastairp
oh right, of course!
2021-04-12 10252, 2021
Mr_Monkey
That's the LastFM calendar view you were talking about alastairp
2021-04-12 10215, 2021
ruaok
I did mention that I love timescale, right?
2021-04-12 10215, 2021
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
2021-04-12 10258, 2021
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
2021-04-12 10258, 2021
Mr_Monkey
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
2021-04-12 10211, 2021
shivam-kapila
ruaok: a good amount of times :)
2021-04-12 10244, 2021
alastairp
Mr_Monkey: hmm, why is the frontend doing multiple queries? in case it gets fewer than 25 listens in a single query?
2021-04-12 10253, 2021
ruaok
yerp
2021-04-12 10256, 2021
Mr_Monkey
Yes
2021-04-12 10258, 2021
alastairp
ah, wait
2021-04-12 10214, 2021
alastairp
I was going to say "but how will we know that 1 more query back will get enough items?"
2021-04-12 10218, 2021
ruaok
there is a fair bit of logic of getting around the limitation. and now we can get rid of all that. #deletecodefuckyear
2021-04-12 10221, 2021
alastairp
but that's not an issue, because you have counts
2021-04-12 10231, 2021
ruaok
yep.
2021-04-12 10234, 2021
alastairp
so you just go back in time until sum(listens) is as many as you need
2021-04-12 10201, 2021
Mr_Monkey
Exactly
2021-04-12 10233, 2021
alastairp
does this also mean that we won't have to store counts in redis and they'll no longer get out of sync>
2021-04-12 10234, 2021
CatQuest
alastairp: oohhh I'll make a ticket of that then
2021-04-12 10234, 2021
alastairp
?
2021-04-12 10248, 2021
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
2021-04-12 10253, 2021
CatQuest
?
2021-04-12 10220, 2021
alastairp
CatQuest: yes, the ? was to be added to my previous line
2021-04-12 10228, 2021
CatQuest
ooohhh hah
2021-04-12 10243, 2021
CatQuest
(I was wodnering why you where cnfused only 5 minutes later)
2021-04-12 10225, 2021
alastairp
_lucifer: hi, I'm looking at the last items of the startup PR now
