TOPIC: MetaBrainz Community and Development channel | MusicBrainz non-development: #musicbrainz | Channel is logged; see https://musicbrainz.org/doc/IRC for details | Agenda: Reviews, GSoC (ruaok), MBS-11371 (reo), CB-408 (alastair)
yvanzo
mo’’in’
ruaok
moooin!
yvanzo
alastairp, ruaok: Which containers are publishing to/consuming from RabbitMQ for ListenBrainz?
I made the change. regarding the integration test, I haven't written one for LB before so that can take some time.
ruaok
actually, I am now questioning this PR.
we've added another serialization/deserialization step in order to rename a key.
that is rather expensive, especially since its in a loop. this whole data ingestion needs to be improved with protobufs at some point.
for now, I wonder if we should ask Mr_Monkey to simply look at track_metadata keys and if that fails, look for a data key.
_lucifer
yeah. for the time being, we could do this client side if ti makes sense?
ruaok
coupled with docs and a ticket for improving this.
yes, exactly that.
if you can make that change, that would be great.
Mr_Monkey: thoughts?
_lucifer
yeah sure, i'll do that.
ruaok
thanks.
_lucifer
another thing to note, this was probably the bug breaking follow feature on client side. once this is deployed, the follow feature might start working.
ephemer0l joined the channel
ruaok
yep, I would expect that. :)
_lucifer
PR updated. I have tested the changes locally and they work. Mr_Monkey, can you please take a look as well ?
alastairp: I think BU-13, BU-14, BU-15 can be closed. thoughts?
_lucifer: the change in 1284 looks fine. Can you please add a test :)
_lucifer
shivam-kapila: sure, i haven't worked with jest before tohugh. is there a websockets test somewhere i could look for ?
shivam-kapila
Hm. We dont need a websocket test imo. Just a json comparision test would be fine I guess. It sends a websocket listen to receiveNewListen and sees if the keys have been properly serializes to make it a listen
I will take a look if we have a similar test somewhere.
_lucifer
great, thanks!
alastairp: closed those issues. let me know the details of the new sentry project when its ready and i'll proceed to the testing.
and BU-32 actually shows up during BU tests. maybe some recent change made that happen. i'll fix that.
I have access to cb and cb-beta but not sure if I am in the cb team
alastairp
I think it's the team that gives access
yes, I just checked. you seem to have it
_lucifer
i am able to access the link you shared
alastairp
cool, so just use that for any testing that you need. Add a note to a PR to remind us that we should delete it afterwards
_lucifer
👍
CatQuest
skjer'a?
_lucifer
alastairp: i was looking at sentry migration guide and did a livegrep to find the usages. I see we also use BU in MeB website but we forgot to upgrade/test it like other apps.
alastairp
MeB already uses new consul
and ruaok force downgraded pip to not get the install error
yeah, we should also upgrade and test it
_lucifer: btw, while upgrading raven to sentry, can you also remove the email logging from BrainzFlask? Nothing uses it, and it doesn't give us anything extra that sentry can do