-- BotBot disconnected, possible missing messages --
-- BotBot disconnected, possible missing messages --
BrainzBot joined the channel
SothoTalKer has quit
SothoTalKer joined the channel
[listenbrainz-server] 14amCap1712 opened pull request #1984 (03crossvalidation…switch-playcounts): Add option to switch between playcounts and transformed listencounts https://github.com/metabrainz/listenbrainz-serv...
[listenbrainz-server] 14amCap1712 opened pull request #1985 (03switch-playcounts…custom-confidence-func): Apply a transformation function on listen counts before calculating confidence in CF https://github.com/metabrainz/listenbrainz-serv...
As you mentioned in CB-434, we don’t need all these tabs. Also since we don’t have anything called “other” in our database, we can just have a parameter `include_events_with_no_type`.
Then in CB, when `other` tab is called, It would request all other event types like Convention/Expo, Launch event, Award ceremony, etc. with parameter include_events_with_no_type as True from BU. So in this case, I don’t feel that having a default case, and custom True and false case would be of much use.
Also for CB, as mentioned, that we’ll have a tab for Stage Performance. I don’t think it is a good idea because when I checked in the MB database, there were only 130 stage performance events.
alastairp
hi
yeah, regarding Stage Performance, reosarevok suggested it as a "perhaps we could add it", but if there are so few, let's omit it
so what do you think that the behaviour of the "events with no type" should be?
So when the parameter is true, it would include events with null type and the event for list passed.
alastairp
and what would you set the default value of the parameter to?
Ansh
False
So basically in CB, for Concert and festivals, the value would be false. For other events, a list of event_types would be passed, and the parameter would be true.
Ansh: right, let's try that then. it'll be simpler than what I suggested
Ansh: could you add a reply to that comment saying that we discussed it and decided on a different behaviour (so that in the future if we want to remember why we did it like that we have a record of the discussion)
Ansh
Okay i'll reply to it
mayhem
lucifer: on #1976 what is the parallelism parameter?
lucifer
mayhem: how many trainings to do concurrently. total trainings is nFolds * nRanks * nAlpha * nLambda * nItrerations.
mayhem
that would change if we change the number of nodes in our cluster?
KevlarNoir has quit
lucifer
upto us how much we want to set it to. default is 1. i had tried 3 once and it went fine. there's a probably a limit to how much we can increase it before it OOMs.
you know how there is a trash icon besides events in the feed page, and now there would be an eye icon too. I was thinking of creating a function that would conditionally render, instead of filling the main code with lots of ternary operators
I was wondering, what should I name the function, something like `eventSideButton` should work right?
odnes has quit
dusjekatt is now known as CatQuest
BrainzGit
[listenbrainz-server] 14amCap1712 merged pull request #1984 (03master…switch-playcounts): Add option to switch between playcounts and transformed listencounts https://github.com/metabrainz/listenbrainz-serv...
Etua joined the channel
lucifer
mayhem: monkey: alastairp: any PRs to merge? will do a release soon
alastairp
no, don't think so. although this week I want to finish the py3.10 pr and the cb reviews one
lucifer
sounds good 👍
BrainzGit
[listenbrainz-server] 14amCap1712 merged pull request #1985 (03master…custom-confidence-func): Apply a transformation function on listen counts before calculating confidence in CF https://github.com/metabrainz/listenbrainz-serv...
oh yeah, I saw your approval. I tested it locally with my own consul server. do you want to go ahead and try it?
lucifer
but that'll require to brind timescale down no?
alastairp
sorry, I don't mean "try that the bring-down functionality works", just "let's try and release it"
lucifer
(i do not have a local setup for consul currently so need to do on test)
ah ok 👍
alastairp
but you're right, we could deploy on test and change the config
lucifer
yup can change the config there but the scenario that broke it last time won't be triggered if timescale is up
BrainzGit
[listenbrainz-server] 14amCap1712 merged pull request #1980 (03master…consul-config-listenstore): Use consul-template if-statement to check if the listenstore is up https://github.com/metabrainz/listenbrainz-serv...
alastairp
right. that shouldn't happen any more. if consul setting is "true" then we get real config, if it's "false" then we get empty config. we'll never get SERVICEDOESNOTEXIST if the config is "false"
so we don't need to test shutting down the timescale server
lucifer
makes sense
alastairp
let me prep a PR for docker-server-configs to change the setting values to json bool
lucifer
i'll release first on beta anyway to make sure config loads fine.
thanks!
alastairp: another question, is there any use of getattr here if the second arg is always fixed. getattr(payload, 'data'), this should be equal to payload.data afaiu but there are many instances of such code in LB so wanted to confirm.
alastairp
if payload.data doesn't exist on the payload object, then that's an exception. getattr will return None instead
in which case is this used? if it's on dynamic data then maybe getattr makes sense? but if it's a fixed class that we control then maybe not
lucifer
but there is no fallback defined so getattr will raise exception too for this. like:
in which case, my feeling is that there wasn't a clear reason to do it like this, maybe we just saw that there was a 'string' and wanted to keep it like that
lucifer
i see, makes sense. i'll remove these redundant getattr's then. they have always irked me ;)
Etua has quit
alastairp
I guess it makes sense if you need a name which changes. e.g. `arg = 'artist_mbid' if do_artist else 'recording_mbid'; getattr(foo, arg)`
but if it's always known, then let's just get the objects directly
lucifer
yup right.
alastairp
lucifer: docker-server-confs PR made, affecting beta and test
if you merge that then release, then you should be able to test it
change true -> false to take down listenstore
once you're ready to deploy on prod, make the same change to the prod config file and deploy it at the same time
I'm going to go and look for some lunch, back soon
mayhem: with the latest listened at stuff in recommendations, we can add a troi config or patch to filter out recs that have been already listened in past X days.
then create a playlist of only not listened ones.
mayhem
I was wondering if we should remove the listened tracks from the candidate sets on the spark side of things.
the idea would be to see if there is no query parameter set to show the type of event - first try and show Concerts, but if there are 0 left, then try and load festivals, but if there are 0 then show the "Other" tab
reosarevok
I think that makes sense (for all entities, so if you have no albums I'd show singles, too)
yvanzo: wtf "08006 DBI connect('dbname=musicbrainz_test;host=localhost;port=5432','musicbrainz',...) failed: FATAL: role "musicbrainz" does not exist"
Do we need to build some new selenium something-something?
yvanzo
Not sure where does it come from but it is actually not a fatal error.
reosarevok: My guess is that some PR that has been merged was missing to update some Selenium test as well.
reosarevok
Does it always fail on this starman thing?
I will check
Seems so
Ok, checking locally
yvanzo
Thanks, I cannot run Selenium tests locally atm.
monkey
riksucks: Hi! Yes, I think extracting that code in a separate method would make everything clearer and more readable. Maybe `renderEventActionButton` or something?
lucifer
LB prod updated.
monkey
Thanks lucifer, sorry I forgot to answer. Will have PRs ready soon but none today :)