Either that, or hide it, which I do'nt think is a great idea
2022-09-21 26405, 2022
alastairp
or "Created for", or we could decide that "Created for you" is a noun, and it's implied that this means that it's the lists created for that specific user
2022-09-21 26406, 2022
monkey
I don't know if there's a lot of room for misinterpreting the tab. It does say the name of the user in that blurb
2022-09-21 26431, 2022
alastairp
yeah, and the sub heading below does directly say what it is, anyway
2022-09-21 26433, 2022
mayhem
if the user sees the page, we can say "created for you". But if another user sees it "Created for [monkey]".
2022-09-21 26453, 2022
alastairp
mayhem: sure, easy to do. I wonder what our longest username is
2022-09-21 26458, 2022
monkey
We do that in a couple of places where there is more room for misinterpretation
lucifer: some good things in that dumps review that we could probably roll back into regular dumps too
2022-09-21 26448, 2022
lucifer
yes makes sense
2022-09-21 26430, 2022
monkey
aerozol: I was thinking of two things we could do together when we are in person:
2022-09-21 26430, 2022
monkey
1. I can show you how I use github desktop and other technical bits of our workflows that you would like to have an intro to
2022-09-21 26430, 2022
monkey
2. we can do a serious session of 4-hands designing/doodling for everything we can think of, from landing pages to LB user dashboard, etc.
2022-09-21 26416, 2022
monkey
1 is for your convenience, and 2 is for maybe preparing projects for you to work on for the next 6 months or so?
2022-09-21 26447, 2022
alastairp
4 hands... any more and I'll start having Cthulhu nightmares
2022-09-21 26408, 2022
BrainzGit
[listenbrainz-server] 14amCap1712 merged pull request #2154 (03recommendations-support-mbid…recommendations-support-mbid-fe): Enable recommend track button if only recording mbid is present https://github.com/metabrainz/listenbrainz-server…
2022-09-21 26449, 2022
alastairp
lucifer: hi, about sqlalchemy 2 warnings - specifically the one about _mapping or .mappings()
2022-09-21 26406, 2022
alastairp
is there still an open PR for this?
2022-09-21 26409, 2022
alastairp I'm getting a bunch of warnings in db/playlist, I'm touching this code now so can fix it if all of the other places have been updated
2022-09-21 26404, 2022
alastairp
hmm, that being said, I see calls to .mappings() in the playlist db method, weird. let me dig into this a bit more
2022-09-21 26432, 2022
lucifer
anope. some of the warnings were introduced later or missed. i haven't opened another PR to fix these because warnings until those are turned to errors in tests, it'll happen again. we cannot currently turn warnings to errors because mbdata needs to be updated.
2022-09-21 26452, 2022
lucifer
i do have the changes locally though probably. let me see.
2022-09-21 26401, 2022
alastairp
👍 ok, I'll fix warnings as they appear in the files that I'm touching
2022-09-21 26414, 2022
alastairp
sure, if you have any for lb/playlist.py send me a diff and I'll apply them
[listenbrainz-server] 14MonkeyDo merged pull request #2147 (03master…add-data-foobar200): Add foobar2000 player and its plugin to compatible players list https://github.com/metabrainz/listenbrainz-server…
2022-09-21 26458, 2022
BrainzGit
[listenbrainz-server] 14MonkeyDo merged pull request #2156 (03master…monkey-listen-duration-seconds): LB-1126: show track duration properly on ListenCard https://github.com/metabrainz/listenbrainz-server…
alastairp: we didn't reach a conclusion earlier. do we want to go the replication packets way?
2022-09-21 26441, 2022
alastairp
lucifer: from what I understand then, using the last_updated will work if any metadata has changed, or if metadata in related items has changed (as we follow these relations)
2022-09-21 26458, 2022
alastairp
but it won't work if other relations are added or removed?
2022-09-21 26416, 2022
lucifer
it can work for added, will need to confirm but definitely not for removed.
2022-09-21 26441, 2022
alastairp
and how long is a dump taking for you now?
2022-09-21 26425, 2022
alastairp
to confirm - what is your artist_mbids test doing here? This is for when a new artist is added to a recording?
2022-09-21 26415, 2022
lucifer
ran from 13:01:20 to 14:41:24
2022-09-21 26446, 2022
lucifer
this is comparing the that all artist mbids in the old and new cache are same.
2022-09-21 26408, 2022
lucifer
this is for checking we are not missing capturing any change.
2022-09-21 26449, 2022
alastairp
so for artist mbids, for changes that happened in MB between the time you generated the 2 tables, we are capturing all changes?
2022-09-21 26421, 2022
alastairp
regarding you actual question: if there are changes that we can't determine based on the last updated field, then we definitely need another way of getting it. I'm not sure if it's best to do that with a replication packet, or by looking at the edit table
2022-09-21 26414, 2022
alastairp
are changes to the edit table included in replication packets? reosarevok? I guess they will only apply if you made a mirror that includes edits in the initial import?
2022-09-21 26401, 2022
monkey
alastairp: Looking at tickets now, turns out the rating is not optional for mini-reviews in LB ?!
btw, another thing I noticed is that the text on this popup says "are licensed under a Creative Commons Attribution-ShareAlike 3.0 Unported or Creative Commons Attribution-ShareAlike-NonCommercial license as per your choice above"
2022-09-21 26449, 2022
alastairp
but doesn't ask for a license choice
2022-09-21 26413, 2022
alastairp
ahhh,
2022-09-21 26419, 2022
monkey
And indeed the LB backedn does set to 0: `rating=metadata.get("rating", 0)`
2022-09-21 26430, 2022
alastairp
that error "Parameter too small" makes sense now, because I see that LB uses pydantic
2022-09-21 26435, 2022
alastairp
and that's a pydantic validation erroro
2022-09-21 26411, 2022
alastairp
`class CBReviewMetadata` doesn't indicate that review and rating are optional
2022-09-21 26423, 2022
alastairp
I suspect it might be this. I can have a look at the validation now if you want
2022-09-21 26448, 2022
monkey
Yes please :) No need to do it right now, just wanted to understand where the issue was
monkey: btw, I see the yellow stars offset from the grey ones
2022-09-21 26458, 2022
monkey
Ooh, floating stars
2022-09-21 26459, 2022
alastairp
do you see that too?
2022-09-21 26417, 2022
monkey
Affirmative
2022-09-21 26457, 2022
alastairp
due to `#CBReviewModal .rating-container .rating-stars>span` `top`
2022-09-21 26435, 2022
alastairp
although that should apply to both child divs, not sure why it isn't
2022-09-21 26422, 2022
monkey
I'll investigate that one
2022-09-21 26426, 2022
monkey
There is an override of the `top` property adde to the element's `style` (so overruling any css definition)
2022-09-21 26455, 2022
alastairp
but only for the gray one, and then the yellow ones aren't overridden also?
2022-09-21 26438, 2022
alastairp
humm
2022-09-21 26411, 2022
alastairp
monkey: does an LB user page make a lookup to api /1/metadata/lookup ?
2022-09-21 26416, 2022
alastairp
yes, `APIService.lookupRecordingMetadata` in now playing popup
2022-09-21 26457, 2022
alastairp
in development, if your api root is set to your local environment, then this endpoint requires a metadata lookup, which requires the mapping tables, and so you get quite a significant error message
that API endpoint should have a bit more error handling
2022-09-21 26423, 2022
monkey
server-side or client-side?
2022-09-21 26435, 2022
alastairp
ah - in this case yes, because I have debug mode on, and that shows a custom flask error interface
2022-09-21 26414, 2022
alastairp
so I think it makes sense to have a way of indicating that in development this endpoint doesn't work, in any case we should probably capture and handle this exception anyway
2022-09-21 26442, 2022
alastairp
right, I guess what's happening here is that the frontend is correctly handling a non-200 status code, but printing the response in a popup
lucifer: looks like somehow the `init_rabbitmq_connection` isn't getting called?
2022-09-21 26430, 2022
monkey gotta go
2022-09-21 26450, 2022
alastairp
how does the existing mapper work then? 🤔
2022-09-21 26453, 2022
alastairp
monkey: will do
2022-09-21 26413, 2022
lucifer
alastairp: i fixed it in some branch. prod is not running master affair
2022-09-21 26423, 2022
alastairp
lucifer: oh duh. .start does init and then calls self.run
2022-09-21 26440, 2022
lucifer
right
2022-09-21 26401, 2022
odnes has quit
2022-09-21 26448, 2022
Aheno joined the channel
2022-09-21 26448, 2022
Aheno has quit
2022-09-21 26421, 2022
Etua joined the channel
2022-09-21 26428, 2022
Etua has quit
2022-09-21 26434, 2022
aerozol
Monkey: sounds great. I think we'll come up with heaps to do actually if we set aside some time! Let's do those though
2022-09-21 26447, 2022
elgranRoble joined the channel
2022-09-21 26407, 2022
aerozol
Oh and thanks for the help with those tickets monkey, I'm visiting family for a few days and giving the arm a rest, I'll go through your instructions when I'm back. Genuinely looking forward to figuring out what all that stuff means!
