#metabrainz

Moin!

hi iliekcomputers

ruaok: moon moin

I was looking through a few recent PRs on LB and had some comments, not sure if I should add them to the already merged PRs

let me know if you want some feedback

Ah, don't know why that tagged him, wanted to tag iliekcomputers :p

alastairp: please do! :)

Mr_Monkey: :)

https://github.com/metabrainz/listenbrainz-serv... this one is interesting

there's nothing wrong with returning content with a non-200 status code, but some api guideline docs suggest to always use 200, even for errors

I'm not sure if we have a guideline (and if not, we should have one)

https://musicbrainz.org/recording/4ae1d3e8-5c83... -> 400, returns html

https://musicbrainz.org/ws/2/recording/4ae1d3e8... -> 400, returns xml

hmm.

this endpoint was more about checking the validity of the token, so Unauthorized didn't seemed appropriate to me.

oh, interesting

the user is authorized to send an invalid token, we just check it against the db and return that it is invalid.

OK, I didn't notice that it was on that endponint

tbh, that endpoint feels a bit weird to me anyway... I'm not sure why people can't just use any endpoint and check for an unathorized response

but you're right, the behaviour of this one is a bit more subtle

there was a dev of an app who wanted that endpoint. to use the other endpoints (just to check that the token is valid) they'd have to create fake data etc. (https://tickets.metabrainz.org/browse/LB-377)

LB-377: Add user token query

right, so we have no endpoint which doesn't create data? that's a pity

ohwell, no problem then

that documentation could be made a bit clearer in this case

it'd be nice to not have to parse a string to get the valid/invalid

e.g. it could return {"status": "invalid", "message": "No such token found"}, or similar

because there's a question of how someone uses this response to verify their token - with the current response do they check against the exact string? What if we change the string in the future? (should we have a comment saying to never change this, because it'll break external code?)

right, good point.

this is a reason why status codes might be nicer - we could potentially change the message and the way that the end-client checks would stay the same

in any case, the documentation should include _how_ the end client can tell that the token is valid/invalid

OK, cool

next!

https://github.com/metabrainz/listenbrainz-serv... I'm not sure about this change either - There likely isn't much efficiency difference in using uuid or varchar types, and it means that we turn an implementation detail into something enforced by the database. Is there a chance that some time we might change this token to something that isn't a uuid?

this is similar: https://github.com/metabrainz/listenbrainz-serv..., I'm not convinced that is is a useful change - it means we lock ourselves into a specific format for this token, which may not always be the case

sure, we may never change the format, but changing the type to uuid doesn't bring us any benefits that I can see

mooooin!

mo''in'

moooin

LB hack day is here! I'm excited.

alastairp: I am not sure. it makes stuff a little cleaner. and it makes the schema change easier because of the default values we can set.

and my mail queue didn't asplode overnight.

iliekcomputers: which pull request specifcially?

the first one.

outsidecontext: we had very few bug reports for Picard 2.1.0, what do you think about releasing 2.1.1 with current fixes ? Also how can we improve things regarding PICARD-1447 ?

PICARD-1447: When releasing a new version, appdata should also be updated https://tickets.metabrainz.org/browse/PICARD-1447

i don't really have a big preference here though.

the fact that we were storing uuids in a string column bugged me, but the argument for future use does make sense.

I'm not sure if that's strictly true - can always do some additional code in the sql update script to add a column, set a value, and then change its default

efficiency-wise, there's not a lot of difference in storing a string or uuid type in postgres

Mr_Monkey: have you read through the doc yet. I made lots of changes after you called for bed time

there's no reason why that column couldn't start as the same value as musicbrainz_row_id

mmm, in fact - thinking out loud: https://flask-login.readthedocs.io/en/latest/#r...

> This way you are free to change the user’s alternative id to a new randomly generated value when the user changes their password, which would ensure their old authentication sessions will cease to be valid

keep in mind that as the code is now, we'll log everyone out when we deploy

(does this actually matter? if we already log out on browser-close anyway, it's not like we have long sessions)

anyway, it's not going to be the end of the world if we make these changes - they just struck me as unnecessary when I saw them

yeah, that sounds good to me. put musicbrainz_row_id's inside the column initially.

it's not going to break anything if we keep them as-is

ruaok: got coffee, will read now

I'd prefer to close the remember me PR soon. it's been there for a long time.

do you think moving forward with it for now is okay? (considering we'd have to request more changes undoing the last request of changes... (oops))

we can open a ticket for changing the column type.

I was going to make a comment about the use of the API, in response to the question

I've got lots of things on the first half of this week, but I'll see what I can do

the thing about the column type, is we'd have to make the change before releasing it - otherwise the use of musicbrainz_row_id is useless (since any time you change the id, you'll log out users)

was https://tickets.metabrainz.org/projects/LB/issu... discussed in IRC at all?

LB-416: Change data type of auth_token

alastairp: yeah.

ruaok: Mr_Monkey: do we start soon? :D

I've been waiting for things to calm down in the channel before taking over.

but, head over to the doc and read what I've updated last night.

there are loads of open questions.

ruaok: I'm basically done, I think

alastairp: ok. thanks!

let me just read what iliekcomputers linked

iliekcomputers: right - I implemented that. At the time, I needed somthing to fill the column with, so I used a uuid. think of it as "a string that happens to be filled with a uuid", rather than "a uuid in a string column". If I'm honest, I was never happy with using uuid to fill that column, so I guess it's a bit like the opposite of that discussion

again, no strong feelings either way, but my preference is to keep it as a string

and on that note, I have other things that have to be finished today. enjoy the hackday!

thanks for the feedback alastairp!

Mr_Monkey: iliekcomputers : let me know when you've read the doc, so we can go over it from the top. make sure we're on the same page.

Yep, all read for me ruaok

ready.

aight.

from a 20,000ft perspective, I'm suggesting that we add a simple web sockets layer that pushed new listens to the client.

the new listens that get pushed are picked from the firehose of listens based on a follow-list sent to the server.

the client can then make intelligent decisions on what listens to play for a user.

the goal is to make the listens page dynamic and to allow the user to explore the listens of another users.

either passively, or following users actively.

decent summary so far?

Yep

hmm.

whatchat got, iliekcomputers ?

where do you choose which listens are to be sent to which connection?

in a flask socketio endpoint?

yes.

if so, where do we get the listens from? Inside influx?

so, I envision that we need a new container.

that container is running at hetzner and follows a structure very similar to a bigquery-writer.

it consumes the de-duped stream of listens.

it keeps track of user -> follow list pairs in redis.

so, when a listen comes is from a user, the inverted list of all the users who are following that user is fetched from redis (or kept in ram) and listens are sent to those websockets.

oh.

we'd still need to watch redis for listening_now listens.

ok, cool.

those are not put into the pipeline.

I'll need some clarification on that, ruaok, but it'll be better in person.

ok.

how are we on general overview?

next, let pick off point 3 near the top.

If a user plays down the page, eventually the user will run out of listens.

what do we do then?

load more listens from the next page.

?

^

1) dynamically load more from the API

2) reload the page to the next page.

2 might be harder, since we need to preserve state between reloads.