#metabrainz

oh, I see.

I was hoping to make this endoint more generic than that.

Sure. For other potential uses I think returning an html string is the most flexible way we'll have.

ok, then I'll leave that feedback on the PR.

thx

Well, we've also got specific icons for each event type, which has all the reasons to live in the react component.

hmmm.

But we could have a generic event type (and icon), and if there's a message associated to that event we parse and render the html.

yes, agreed. lets do that.

ruaok, Mr_Monkey: what was finally decided for the endpoint? just keep a link and another endpoint for other generic events?

I'm lacking context to answer, haven't looked at the PR in question at all

But we agreed that for generic events the most flexible solution is to send html as a string and parse it on the front-end side

_lucifer: we agreed that we should only have a `message` field and remove the link field.

👍

i'll update the PR accordingly.

the message should be HTML and then sanitise it to ensure that no bad HTML is in there.

thanks!

from a friend of mine and friend of MB: https://r0ml.medium.com/free-software-an-idea-w...

ruaok, sanitizing with the same constraints as in playlist_api https://github.com/metabrainz/listenbrainz-serv... is fine or do we want different constraints for this endpoint?

same is great.

👍

We can also sanitize on the front-end for double safety

We already use a library for that in another page

once should be fine, since this is not a public endpoint.

only if the submitter token is compromised might this code ever kick in.

ruaok, PR updated.

Mr_Monkey, if you are available should we discuss the UI changes mentioned for Youtube Player Implementation.

Mr_Monkey: https://github.com/metabrainz/listenbrainz-serv...

that is the PR that allows notiifications to be posted to users timelines. can you have a look and examine how we're doing on icons for that event?

_lucifer: I'll have a look at 1342 first and we can discuss youtube stuff afterwards id it works for you?

ruaok: I'll open a separate PR to add support for the notification events

Mr_Monkey: +1

[listenbrainz-server] MonkeyDo opened pull request #1359 (master…monkey-notification-timeline-event-ui): Add support for 'notification' events on timeline UI https://github.com/metabrainz/listenbrainz-serv...

I still have to add some tests, but ^

_lucifer: Available when you are :)

So, does PR 1342 mean that we won't have specific playlist_created events anymore?

Mr_Monkey: I believe so, yes.

OK, my PR should also remove that then

bitmap, reosarevok: https://tickets.metabrainz.org/browse/MBBE-39 is up for review.

MBBE-39: Remove invalid characters from existing annotations

Mr_Monkey: Hi!

Yo !

I saw our youtube player implementation, it seems to me all we need to do is change a network on that front.

For the search you mean?

yes

Yes, with some extra new bits to ensure the user has a token (like we do in SpotifyPlayer)

we need to replace the `loadPlaylist` with a call to data api and pass that to the player and should be done.

yes agreed.

but we don't need to change anything in the UI?

Not for the youtube player itself, no

understood.

moving on, what UI changes do you think we need to for other players?

in context of "Discuss potential UI changes with Monkey in order to reduce the UI work that needs to be done by potential GSoC students."

We need to rework the existing page where we connect to Spotify, is the short story. Instead of being a specific "connect to spotify" page we're going to want to allow various music services

I did say I was going to make some mockups for that, and I haven't done that yet

ah yes that makes sense. I had forgotten we need to change the UI for that page as well :p

Yeah, and it's going to need a good refactor.

I'll get some mockups done which we can then use as a starting point to talk about

At hte moment I have a few ideas in my head, but nothing on paper

yeah right, I'll try to get the player part working this week and take care of connecting at start of next week.

cool thanks!

*connecting music services

I'll amend the document too. I wrote "This screen should also allow users to disable playback for each music service independently.", but I'm not convinced we want to do that on this page.

makes sense

I quite like the screenshot at the end of the "fron-end" subsection in the document, but I'm not sure we'll be able to both explain the various uses we make (e.g. for spotify record listens vs play music) and player preferences without it getting confusing.

So i'll work on a specific "connect to music services" page and we can see about the player preferences another day

yeah, its already complicated to handle as a developer. it could be a nightmare for users if put together.

different pages sound good to me.

Can the "add musuc services doc" be shared with me? I side by side work on the dashboard redesign and if there are some changes to BP I will keep those in mind

No changes to BP in all of that

I mean no visual changes, just some implementation changes in the youtubeplayer

Cool thanks for the info

ruaok: regarding https://stats.metabrainz.org/d/000000079/alerts... , the queue's size seems to follow a regular pattern. it exceeds the limit to issue an alert once everyday. would it make sense and be possible to either increase the limit at which the alert occurs or if the queue size remains above the current limit for a period of time?

zas manages those alerts, but I agree.

unrealted but also i have a couple of doubts regarding consul and services on lemmy.

i saw this https://sentry.metabrainz.org/metabrainz/listen...

ruaok, _lucifer : I bump the alert threshold to 25k

zas thanks.

_lucifer: yeah, I have no idea what causes that.

thanks zas!

ruaok, i am checking the logs of spark reader and comparing it with timescale writer to see if i find something. will let you know if i find something.

_lucifer: https://github.com/metabrainz/listenbrainz-serv... is ready to merge then, yes?

hi, I'm back. will be here for an hour-ish

hi back. nice to meet you.

now I'm angry

look at how quickly I can change!

ruaok, yes. i also made one change after you approved, it now raises `APIForbidden` for non approved users.

damnit, why can't we have ser/estar in english

I saw, I was just checking. ok.

> damnit, why can't we have ser/estar in english

you really consider that a feature?

at least then you wouldn't be able to make dad jokes

[listenbrainz-server] mayhem merged pull request #1342 (master…troi2): Add api endpoint for approved bots to post on user's timeline https://github.com/metabrainz/listenbrainz-serv...

_lucifer: have a look at this so we can totally finish this feature? https://github.com/metabrainz/listenbrainz-serv...

on it.

It seems https://musicbrainz.org/edit/78344687 is the last cover art upload that redirects.

Everything after that gives 404: https://musicbrainz.org/search/edits?auto_edit_...

chaban: Everything is working for me.

No 404 for this one? https://coverartarchive.org/release/a1c0dd45-7c...

Oh ok, no it isn’t, thanks for reporting!

This is bad. Picard also can't download cover art for newer uploads

bitmap ^ caa-redirect containers’ logs aare not reporting any issue, might it be an issue with the IA?

caa-indexer only reports succeeded uploads too.

CAA-127: Cover Art Archive redirects don't work for uploads newer than Edit #78344687

alastairp: metric names of `listenbrainz:mbid-mapping-writer` vs `listenbrainz-mbid-mapping-writer`. Not sure I understand your argument for not exactly using container names. our container names are nicely separated in the namespace....

ruaok: it's mostly a question of how we store the data in redis and how we retrieve all stats in the internal endpoint

we're going to want to say "give me a list of all listenbrainz stats"

so if we store a key in redis `listenbrainz-mbid-mapping-writer = whatever`, we need a way of knowing that this is a listenbrainz key (without manually iterating through all keys and doing string matching)

ah, for the redis keyspace... whatever. that doesn't bother me since we won't be interacting with it. I only care that we really get the metric names right.

ok sure, this is an implementation detail. keys can be listed in the json exactly as they are in the container name

what do you think of my suggestion of appending some further qualifier, like "module" or "component".

because one container may want to report stats in two separate areas.

I think I had this in the initial version and we removed it because we thought it was too complex ;)

do you think `key: listenbrainz-mbid-mapping-writer:foo` and value: 4, or `key: listenbrainz-mbid-mapping-writer`, `foo:value: 4`

bitmap: should be the IA, there seems to be nothing wrong on our part: https://stats.metabrainz.org/d/000000075/alerts...

alastairp: I find that I can think better in context of the gist we've been bantering about. can you update that with this question/suggestion?

yep, will do

regarding host name, there's no env variable with this set in the startup scripts, but we set the docker container hostname to the service that it's living on. any preferences to `socket.gethostname()` or passing it in as an env variable?

zas would be best to answer this, methinks.

👍

my personal pref would be to use gethostname, rather than passing in more info.

yvanzo: there's no redirect performed (it's returning a 404 from our own database), so doesn't appear to be the IA

and these IDs aren't in the cover_art table...

ok then there is also an issue with errors logging.

uhh ok so they are in the cover_art table (on floyd), but not in pink. there appears to be some serious replication lag. looking into it

temporarily disabling postgres-pink in consul

there's no indication of why the lag is occuring in floyd or pink logs, unfortunately

I restarted pink and it's slowly recovering, but so far can't find why it stopped

*restarted postgres-pink

Good catch!

bitmap: I just added an alert for this: https://stats.metabrainz.org/d/Ha7ejswMk/alerts...

zas: thank you, was going to ask for that :)

I did on wal count

set to 500 (for pink & floyd, each)

Some major OpenSSL vulnerabilities announced today, CVE-2021-3449 is pretty bad by the sounds of it (remote DoS). All OpenSSL 1.1.1 versions are affected. If you are vulnerable you will want to upgrade ASAP.

That's a job for… super zas ! *trumpet sound*