#metabrainz

/

      • MRiddickW has quit
      • MRiddickW joined the channel
      • sumedh joined the channel
      • adhi001 joined the channel
      • sumedh has quit
      • sumedh joined the channel
      • joshuaboniface has quit
      • yyoung has quit
      • atj has quit
      • joshuaboniface joined the channel
      • atj joined the channel
      • yyoung joined the channel
      • RetroPunk has quit
      • RetroPunk joined the channel
      • prabal joined the channel
      • BrainzGit
        [musicbrainz-server] reosarevok merged pull request #2045 (master…MBS-11566): MBS-11566: Ensure consistent ordering of appearances sections https://github.com/metabrainz/musicbrainz-serve...
      • yvanzo
        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?
      • BrainzBot
        MBS-10566: Make allowed_(collection|series)_entity_type constraints table-based https://tickets.metabrainz.org/browse/MBS-10566
      • yvanzo
        (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.
      • BrainzGit
        [musicbrainz-server] reosarevok opened pull request #2054 (master…MBS-9631): MBS-9631: Add 1200px to available image sizes https://github.com/metabrainz/musicbrainz-serve...
      • reosarevok
        yvanzo: well, see what bitmap thinks :) If he's ok with code constraints only, then so am I
      • yvanzo
        reosarevok: about #2050, would you prefer commits to be rearranged, or just squash them all (and rearrange commit message) at merge time?
      • reosarevok
        yvanzo: we're missing genre and URL
      • For collections
      • That seems to be all, all others are already allowed.
      • Does it make sense to even have restrictions given we already allow pretty much everything, I dunno
      • Re: #2050 I'd just squash and change commit message. Whether at merge time, or yyoung can do it before if they want to :)
      • yvanzo
        The entered entity type still has to make sense, no 'bird' collection will be ever allowed I guess ;)
      • reosarevok: so #2050: no preference
      • reosarevok
        Sure, I guess we don't have an existing "CoreEntity" constraint at the DB level we could reuse
      • #2050: preference is it gets squashed but when, no preference
      • yvanzo
      • reosarevok
        This guy is the same who was spamming our Jira
      • Can we block him from github?
      • yvanzo
        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
      • BrainzBot
        MBS-11579: Renderer tries to load fictitious localization file https://tickets.metabrainz.org/browse/MBS-11579
      • reosarevok
        Like, how it happens
      • BrainzGit
        [musicbrainz-server] reosarevok opened pull request #2055 (master…MBS-1658-impl): MBS-1658 (Impl.): Add free comment field to collection entries https://github.com/metabrainz/musicbrainz-serve...
      • reosarevok
        Draft!
      • I wish brainzgit said it :p
      • Hmm
      • yvanzo: we also show the non-valid message here:
      • yvanzo
        reosarevok: no, I did report everything I have about that bug.
      • reosarevok
        Since that's an existing CDTOC being moved, I guess that should say something else
      • But also like "really, not your fault"
      • I can't imagine this happens often
      • That an invalid CDTOC did get added and is now being moved...
      • Oh
      • Actually there what that means is *the provided CD TOC ID is not valid* so we should reword it anyway
      • (it's like http://localhost:5000/cdtoc/move?toc=31102 and it just checks that 31102 matches an existing cdtoc)
      • So maybe "The provided ID does not match an existing CD TOC"
      • id, I guess we lowercase it elsewhere
      • Or, given we do "The provided medium id is not valid" next, just "The provided CD TOC id is not valid". I'll just do that
      • BrainzGit
        [musicbrainz-server] reosarevok opened pull request #2056 (master…MBS-1588): MBS-1588: Better errors for invalid CD TOC https://github.com/metabrainz/musicbrainz-serve...
      • prabal has quit
      • ruaok
        mooooin!
      • iliekcomputers
        moin!
      • antlarr has quit
      • antlarr joined the channel
      • alastairp
        hello
      • ruaok
        alastairp: hey.
      • 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
      • CatQuest
      • BrainzBot
        LB-864: Newer/Older buttons do not work as links
      • alastairp
        Mr_Monkey: ^
      • thanks CatQuest
      • CatQuest
        no prob.
      • :)
      • alastairp
        ruaok: oh, also - you're right, jasondk's proposal looks execelent. great work jasondk
      • ruaok
        who should respond to this tweet? https://twitter.com/LatDorian/status/1380852476... yvanzo, bitmap?