monkey: "Error while submitting feedback JSON document must contain recording_msid and score top level keys" when I click heart on listening now track on lb prod.
2021-09-17 26048, 2021
ruaok
also mooooin!
2021-09-17 26044, 2021
CatQuest
"[21:17] <ruaok> I am now, lucifer "
2021-09-17 26044, 2021
CatQuest
#messagestakenoutofcontext
2021-09-17 26000, 2021
CatQuest
(sorry :D)
2021-09-17 26057, 2021
monkey
ruaok: I suppose that's expected, the playing_now listens don't have the necessary recording_msid
2021-09-17 26054, 2021
monkey
Buttons should be disabled in that case though. Can you mae a ticekt please?
2021-09-17 26004, 2021
monkey
Actually, never mind, I got it
2021-09-17 26059, 2021
ruaok skips adding the ticket.
2021-09-17 26028, 2021
Etua has quit
2021-09-17 26016, 2021
monkey
Time to refactor the listen component !
2021-09-17 26051, 2021
ruaok
may the force be with you, monkey
2021-09-17 26038, 2021
ruaok
hmm, the not being able to heart a listen_now is... not making for a good workflow. I now have to wait for the song to appear in my listens, which can be delayed by minutes. by then I don't remember what I loved/hated, since its all new to me.
2021-09-17 26048, 2021
ruaok
I'll have to think of how to make that work on the backend.
2021-09-17 26022, 2021
lucifer
now playing listens are not lookup up for msid. since that part was moved to ts writer.
2021-09-17 26006, 2021
ruaok
yes, is the problem.
2021-09-17 26008, 2021
lucifer
we could add msid lookup for now playing listens directly in api or websockets though.
2021-09-17 26047, 2021
lucifer
only api actually (putting in ws will cause inconsistencies)
2021-09-17 26054, 2021
ruaok
that leaves the possibility of having feedback for msids that may not turn into listens.
2021-09-17 26024, 2021
ruaok
if the user clicks hate and skips the track before 30 seconds, we've got shit data.
2021-09-17 26028, 2021
lucifer
right, but we can already love/hate another person's listens?
2021-09-17 26014, 2021
ruaok
we can on test
2021-09-17 26020, 2021
alastairp
how complex would it be to have a "queue" of likes (username, artist-recording credit)?
2021-09-17 26026, 2021
lucifer
which we might not have listened anyways
2021-09-17 26046, 2021
alastairp
and then turn them into real likes as a listen comes in
2021-09-17 26003, 2021
alastairp
I guess that doesn't help it if the track never turns into a listen (user skips before 30s)
2021-09-17 26006, 2021
ruaok
more complex than I care for, really. but that seems to be the right answer.
2021-09-17 26030, 2021
ruaok
perhaps the queue is not the right method here
2021-09-17 26036, 2021
ruaok
redis with a timeout of 1hr?
2021-09-17 26040, 2021
alastairp
or, for a like without an msid, directly do an acr lookup and store it as an mbid like
2021-09-17 26043, 2021
lucifer
i think its fine as is. if you have a listen for msid, the feedback is considered otherwise spark or whatever else ignores it
2021-09-17 26045, 2021
ruaok
lucifer: how is it fine? it doesn't provide a feature I feel is missing.
2021-09-17 26057, 2021
ruaok
acr lookup is an interesting idea, alastairp
2021-09-17 26032, 2021
ruaok
I think my redis idea is doable:
2021-09-17 26010, 2021
ruaok
1. when we get love/hate on now listening, we add an item to redis: (user, artist_name, track_name) with expiry of 1hr
2021-09-17 26032, 2021
lucifer
ruaok: i meant fine as in let people submit feedback as they want regarless of listens. if you give feedback for a track that you haven't listened to it will just get ignored.
2021-09-17 26034, 2021
ruaok
2. when the TS writer gets a listen, it checks to see if that exists in redis. if so, add feedback, remove redis key
2021-09-17 26001, 2021
ruaok
lucifer: ah, I see. but then we have cruft in the DB.
2) we will probably want people to give feedback on their recommendations, not necessary that people listen to those before providing feedback
2021-09-17 26043, 2021
ruaok
yes, good points.
2021-09-17 26010, 2021
ruaok
esp the hate part. we don't want to force people to listen to a track they hate at second 3.
2021-09-17 26004, 2021
ruaok
so, if we add a add-now-listening-feedback API endpoint which provides user, artist, track, we generate MSIDs, and add a feeback event.
2021-09-17 26006, 2021
ruaok
like that?
2021-09-17 26017, 2021
reosarevok
But maybe it gets *AMAZING* at second 5! :P
2021-09-17 26040, 2021
ruaok
your mom is amazing at 5 seconds.
2021-09-17 26027, 2021
lucifer
an endpoint for now playing listens sounds good
2021-09-17 26043, 2021
ruaok
k, let me add a ticket for that.
2021-09-17 26020, 2021
lucifer
and as for the feedback itself, my suggestion is keep that feedback. ignore feedback without listens for the time being. once we incorporate feedback with listens in recs, we can try to figure out to use the one without listens as well.
2021-09-17 26050, 2021
ruaok
agreed. and we could always cleanup feedback later if we find it needs to be done.
LB-950: Add support for love/hate on a now listening listen
2021-09-17 26037, 2021
ruaok
lucifer: another thing we discussed in person yesterday...
2021-09-17 26011, 2021
ruaok
the listen_mbid_mapping table currently uses artist_credit_id, when it would clearly be better to use artist_mbids[].
2021-09-17 26006, 2021
ruaok
and the fact that the MBID mapping tables previously didn't contain all of the releases (it skipped VA releases) and that the old mapping had some minor data instability.
2021-09-17 26029, 2021
ruaok
it would be good to re-generate the table, but named tmp_listen_mbid_mapping.
2021-09-17 26009, 2021
ruaok
once that is caught up, we stop both mbid-mapping writers and swap one table for the other. then restart the new mapping writer as the default one. then drop the old table.
2021-09-17 26016, 2021
ruaok
make sense?
2021-09-17 26031, 2021
alastairp
does pg do table aliases?
2021-09-17 26036, 2021
alastairp
I guess that's basically a view
2021-09-17 26042, 2021
alastairp
and I guess table renames are instant too
2021-09-17 26056, 2021
ruaok
yeah, you can do it all in one transaction.
2021-09-17 26036, 2021
alastairp
ruaok:
2021-09-17 26040, 2021
alastairp
> The DROP COLUMN form does not physically remove the column, but simply makes it invisible to SQL operations. Subsequent insert and update operations in the table will store a null value for the column. Thus, dropping a column is quick but it will not immediately reduce the on-disk size of your table, as the space occupied by the dropped column is not reclaimed. The space will be reclaimed over time as existing rows are updated.
2021-09-17 26009, 2021
alastairp
although since we're append-only we're not going to free any space unless we vacuum full
2021-09-17 26029, 2021
alastairp
(or migrate to a larger server at some time in the future)
2021-09-17 26022, 2021
ruaok
I think that is all fine.
2021-09-17 26027, 2021
alastairp
agreed
2021-09-17 26003, 2021
BrainzGit
[listenbrainz-server] 14mayhem opened pull request #1618 (03master…include-all-releases-in-mapping): Add all releases (not use non VA) into the mapping https://github.com/metabrainz/listenbrainz-server…
2021-09-17 26039, 2021
lucifer
ruaok: yup, makes sense. +1
2021-09-17 26029, 2021
monkey
Something else we talked about yesterday, not sure if that's still not working as intended: LB-493
yyoung[m]: ^ do make sure this seems fine to you when you have some time, please :)
2021-09-17 26042, 2021
reosarevok
yyoung[m]: hmm, can't we just highlight the relationship(s)?
2021-09-17 26053, 2021
yyoung[m]
Sure
2021-09-17 26055, 2021
BrainzGit
[listenbrainz-server] 14mayhem merged pull request #1618 (03master…include-all-releases-in-mapping): Add all releases (not use non VA) into the mapping https://github.com/metabrainz/listenbrainz-server…
2021-09-17 26002, 2021
reosarevok
So, if the user changes the existing one, mark yellow, if the user removes, then red, if adds, then green? Or something :)
2021-09-17 26023, 2021
yyoung[m]
Ideally that would be better, but select boxes are not suitable for highlights
2021-09-17 26045, 2021
reosarevok
But we also have "type: ", right?
2021-09-17 26057, 2021
reosarevok
Why not add a background to the whole div?
2021-09-17 26037, 2021
yyoung[m]
Or what about changing the background of select, like the red one before?
2021-09-17 26039, 2021
yyoung[m]
Highlighting the whole div probably works though.
2021-09-17 26019, 2021
reosarevok
Either works IMO, but the div might be more visible
2021-09-17 26012, 2021
yyoung[m]
OK then, I'll have a try
2021-09-17 26017, 2021
yyoung[m]
And what about highlights of URL?
2021-09-17 26004, 2021
lucifer
ruaok: looking at sitewide stats in spark, it seems the sitewide one is calculating the top artists for each day in past 2 weeks, each day in past 2 months, each month in the past year 2 years and each year since the start. this looks like an overkill to me. we do this sort of stuff during user listening activity calculation which makes sense but for user top artists we do not.
2021-09-17 26005, 2021
reosarevok
Well, I'd expect to highlight what has changed
2021-09-17 26016, 2021
reosarevok
So if the URL has changed, highlight that, if the relationship, highlight that
2021-09-17 26023, 2021
lucifer
i think we should not do it for sitewide top artists either. thoughts?
2021-09-17 26028, 2021
reosarevok
If all uses of a URL are being removed, maybe highlight the whole block in red?
2021-09-17 26050, 2021
reosarevok
But I'd have to see it to be sure it'd work :)
2021-09-17 26035, 2021
yyoung[m]
Yes
2021-09-17 26017, 2021
atj
!m reosarevok
2021-09-17 26017, 2021
BrainzBot
You're doing good work, reosarevok!
2021-09-17 26031, 2021
atj
thanks for fixing those bugs I submitted <3
2021-09-17 26017, 2021
reosarevok
Np :)
2021-09-17 26040, 2021
reosarevok
Not sure when it'll get merged, but I'm hoping release after next (so, beta in two weeks, out in three)
2021-09-17 26033, 2021
yyoung[m]
yvanzo, reosarevok : BTW, do you think displaying type combinations in a dialog is enough for MBS-11899?
Do you mean having a bubble with documentation, or having a select with all the allowed combinations, or a third, different thing? :)
2021-09-17 26023, 2021
ruaok
lucifer: not sure I follow the question. are you suggesting that we only calculate sitewide stats for all time and not the other ranges?
2021-09-17 26044, 2021
yyoung[m]
reosarevok: I mean providing a link in error message for users to view a list of allowed type combinations. My first thought is using a dialog since we don't favor bubbles now.
2021-09-17 26057, 2021
Etua has quit
2021-09-17 26028, 2021
Etua joined the channel
2021-09-17 26056, 2021
BrainzGit
[musicbrainz-server] 14reosarevok opened pull request #2270 (03master…MBS-10056): MBS-10056: Don't add redundant credit to relationship on entity merge https://github.com/metabrainz/musicbrainz-server/…
