not familiar much with sentry's setup but in general looks good!
2021-09-07 25038, 2021
reosarevok
lucifer: the gids are there too :)
2021-09-07 25040, 2021
lucifer
let's deploy it!
2021-09-07 25009, 2021
lucifer
reosarevok: nice, we can use them to extract the relevant data.
2021-09-07 25051, 2021
lucifer
the works' indexing has been running for a few hours on bono now. in progress no errors yet.
2021-09-07 25022, 2021
reosarevok
Ok, let's see
2021-09-07 25059, 2021
reosarevok
I'm guessing we should have only a few entities of each type, plus all relevant artist types, work types, etc :)
2021-09-07 25015, 2021
lucifer
+1
2021-09-07 25021, 2021
lucifer
alastairp: testing out flask-cors, it does not seem to obey views's method parameters. it will just return the methods set in flask config. alternative is to use its @crossorigin decorator but then we are back to where we are now.
so it's not saying "you can only make this type of request to this endpoint"
2021-09-07 25013, 2021
lucifer
yes. i saw that difference too. but Allow is also just a reference right?
2021-09-07 25031, 2021
alastairp
you can definitely try and make a request using a method that isn't in Allow
2021-09-07 25011, 2021
lucifer
right that's my understanding as well
2021-09-07 25057, 2021
lucifer
so ACAM is returning an extended list, including things which actually aren't supported. i think it'll work anyways in this case but just adds more confusion.
2021-09-07 25034, 2021
lucifer
i.e. not use Flask-CORS if we can't stop this behaviour
so, any alternatives? Keep our own decorator? Make an extension of this model to support it?
2021-09-07 25042, 2021
lucifer
keep using our own decorator for now. i'll try to get the contribution in the Flask-CORS extension. it seems they do want it. if that doesn't work out, create our own extension.
2021-09-07 25034, 2021
alastairp
tried to do sentry upgrade, but it supposedly requires newer docker to what we have. Looking into it more to see if we can omit this requirement
2021-09-07 25003, 2021
lucifer
:/
2021-09-07 25013, 2021
alastairp
zas: is it possible to do docker upgrade on serge? Otherwise we might have to move sentry to a more recent server
this is the commit which did it. it's interesting, they're only reporting that it's "possibly" the solution to their problems
2021-09-07 25057, 2021
alastairp
let me see if I can skip the version check
2021-09-07 25013, 2021
alastairp
ok, thanks lucifer
2021-09-07 25045, 2021
opal joined the channel
2021-09-07 25053, 2021
monkey
LB team, another question regarding listens submitted from brainzplayer: what value should additional_info.listening_from have? `listenbrainz`? `brainzplayer`? `https://listenbrainz.org/` ?
2021-09-07 25046, 2021
lucifer
depends. if in future, we allow BP to be embeddable then do we want it to BP for all sites?
2021-09-07 25012, 2021
monkey
There's also origin_url that could help differentiate
2021-09-07 25005, 2021
lucifer
right. but ideally i think there should be one field for source.
2021-09-07 25052, 2021
monkey
What is your suggestion then?
2021-09-07 25021, 2021
lucifer
listenbrainz
2021-09-07 25055, 2021
monkey
That would be my favorite option too
2021-09-07 25056, 2021
alastairp
is this the time to address the ticket that talks about the split between service and tool?
reosarevok: re MBS-960 I do agree using triggers to manage these is the correct/cleaner solution (and feel free to do that if you prefer) but I figured modifying the existing methods might help the PR get merged quicker, so
It probably would, but it feels a bit odd to have the query first find all the entity IDs, then pass it to withdraw or whatever so that it will deal with them one by one (actually, would that even need modifying? In theory it could just be called in a loop, I guess)
2021-09-07 25048, 2021
reosarevok
And the whole thing sucks so much anyway
2021-09-07 25031, 2021
reosarevok
Also, does the PR above seem sensible? IIRC you warned me back in the day that doing it like that would eventually become messy :p
2021-09-07 25037, 2021
reosarevok
So I'd like to avoid a second messy thing
2021-09-07 25020, 2021
bitmap
oh I see, the bug is from the language objects being reused I guess?
2021-09-07 25056, 2021
reosarevok
Yes
2021-09-07 25001, 2021
bitmap
not sure I remember what I warned about specifically :) I just remember saying we have too many of these [No lyrics] fallbacks spread out across the code
2021-09-07 25037, 2021
reosarevok
Me neither tbh, just remember being told to be wary :D
2021-09-07 25032, 2021
alastairp
monkey: right, I like that we can show both of them, then. Do we need to go through existing submission tools and see what they use?
2021-09-07 25007, 2021
monkey
There are basically three fields: "listening_from", "origin_url" and the suggested "source"
2021-09-07 25028, 2021
monkey
I believe it's limited to that
2021-09-07 25035, 2021
yvanzo
reosarevok, bitmap: do you want to decide the next milestone now?
2021-09-07 25038, 2021
alastairp
origin_url is the url of the mp3 file/youtube link/spotify recording?
2021-09-07 25007, 2021
reosarevok
yvanzo: I was going to say let's meet tomorrow but that shouldn't take super long, so we can do it now too if bitmap is up for it :)
We're hoping for releases every two weeks again, right, yvanzo? :)
2021-09-07 25050, 2021
yvanzo
right
2021-09-07 25056, 2021
bitmap
sure, let's do that
2021-09-07 25008, 2021
yvanzo
so next beta on Monday
2021-09-07 25016, 2021
yvanzo
next beta +freeze
2021-09-07 25019, 2021
lucifer
:D
2021-09-07 25023, 2021
yvanzo
(we can update beta earlier)
2021-09-07 25032, 2021
reosarevok
So the milestone should be due on Monday I guess?
2021-09-07 25038, 2021
lucifer
let's move this on to sir-test then?
2021-09-07 25039, 2021
reosarevok
Rather than on release day?
2021-09-07 25045, 2021
yvanzo
yes
2021-09-07 25047, 2021
alastairp
monkey: cool, that sounds good to me. the only other thing that came up on the forums about that was a version for `listening_from`. do you have any thoughts on that?
2021-09-07 25048, 2021
reosarevok
Ok
2021-09-07 25006, 2021
reosarevok
lucifer: should we run a full index and see if anything else broke now?
2021-09-07 25028, 2021
monkey
We could accept either a string or an object of shape {name, version} and any other field considered useful?
2021-09-07 25035, 2021
lucifer
sure, will take almost another day though
2021-09-07 25054, 2021
monkey
as you pointed out in that post though, `version` is a fluid concept
2021-09-07 25059, 2021
reosarevok
I'm pretty sure you have enough stuff to do in the meantime :) (and I'm planning to be off after this talk here anyway)
2021-09-07 25007, 2021
lucifer
works alone took 6 hrs.
2021-09-07 25020, 2021
lucifer
yup, triggered full reindex on bono.
2021-09-07 25035, 2021
reosarevok
Plus, nothing stopping us from trying to put some test data together at the same time, right? :)
my thoughts about this structure is that we said "we're open to what you can submit, just send stuff and we'll see what sticks!" and then everyone just does exactly what we put in the docs. so I'd rather we just explicitly tell people what to put
2021-09-07 25058, 2021
alastairp
and given that we have years of listening_from in a speicfic format, I think I'd rather we just recommended a "listening_from_version" key
If you're ok with that, I'd put the edits still open there :)
2021-09-07 25030, 2021
reosarevok
Then maybe we can stop the special-casing too
2021-09-07 25034, 2021
bitmap
I was gonna suggest putting the edit ones yea
2021-09-07 25020, 2021
yvanzo
+1
2021-09-07 25050, 2021
reosarevok
Ok, I'll start with that then
2021-09-07 25055, 2021
monkey
I suppose we could somehow get the git hash of the current commit, send it to the front-end and use that for the listening_from_version of BrainzPlayer
2021-09-07 25008, 2021
alastairp
listening_from, listening_from_version, source, and origin_url, then? source and listening_from are strings, not URLs. That's backwards compatible with what we have now, too
2021-09-07 25034, 2021
alastairp
yeah. that value is available as an env variable in the webserver container
