#metabrainz

np.

happens to me all the time :)

_lucifer: looks good. don't forget to remove the assert false

yes

is this an issue with other tests too?

yeah, all workflows.

I'm planning to be in the office on Wednesday. should we aim for a PR cleanup?

that would very good.

gotta to be a good feeling for the people who work there

alastairp, also i forgot to mention that the tag workflow didn't work :(. i'll debug why so.

oh, that's sad. let me know what you find

sure.

[listenbrainz-server] amCap1712 merged pull request #1393 (master…gh): Check for CI env variable https://github.com/metabrainz/listenbrainz-serv...

alastairp: fetching min/max timestamps using the cont agg takes nearly 3s. doing it directly from the listens table? 1.1s

mmm, odd

this is past my postgres knowledge

the thing to know: continous aggregates are pretty damn slow.

514k rows in this view. pretty sad.

hmmm. let me ensure that indexes a built for the view

oh wow.

ok, some users are fast to fetch directly. others, not so.

ruaok: your tests are now successfully failing \o/ lol :D

lolol. very good, thanks.

quick question: the `cache.get(REDIS_USER_TIMESTAMPS + user_name, decode=False)`

you need the namespace as well there if you passed one during set

call returns bytes... previously we were using encoding and it returned str()

the namespace is set when the timescale listenstore starts.

so, my keys have namespaces set -- should be ok, no?

let me check and get back that to you

regarding str pass decode=True

what type do you want bytes or str?

str

but I don't really want me keys to be encoded. there is no need.

yes, then pass encode=True and decode=True

encode/decode is for the value, not the key

we don't encode keys any more

understood, for the value.

that change will come in LB once we upgrade BU

(not encoding keys)

maybe we're talking about a few different things

can one of you please tell me the correct and future proof way to set/get keys?

ideally no encoded that we don't have to rescan the DB when new BU is deployed.

pass both encode and decode=True

and that will currently encode my values?

sot that will require a flush of all the values when the new BU comes?

yes that will correctly encode the values.

(which is EXPENSIVE!)

i am not sure regarding the flush because the change is to encoding of keys not values. maybe alastairp can tell about that.

no, you won't need to do anything when a new BU comes

wow, this convo keeps getting worse. ;

we should prbably do the new BU upgrade soon. to avoid such confusions.

the endode/decode flags are to take whatever value you pass into cache.set() and serialise it using msgpack. this means that you get out exactly what you get in

this is the default and the cache has always worked like this

we're not changing this behaviour in the upgrades to BU

yes, but does one return str() and the other bytes() ?

yes, because the redis python library returns bytes. If you don't ask it to decode the value, you get the bytes back

get(..., decode=True) returns a string, get(..., decode=False) return bytes

so, when the decode/encode flags go away we will always return bytes?

sorry, maybe there was a confusion here

decode/encode are not going away

ahhh, ok.

[listenbrainz-server] amCap1712 opened pull request #1394 (master…gh-tag): Test docker image workflow https://github.com/metabrainz/listenbrainz-serv...

alastairp: https://github.com/metabrainz/listenbrainz-serv... i removed the tag pattern and it seems to work. so i think due to some reason the pattern is not matching.

which is strange because i tried in regex101 online and the tag matches the pattern.

_lucifer: I believe that these aren't regular expressions

see that they're called "filter patterns"

it's not clear that it supports `{4}`

yeah this is probably some adhoc github thing..

it worked https://github.com/metabrainz/listenbrainz-serv...

had to remove the quotes

so maybe only use quotes if there is a * involved

great!

does this workflow use the image cache?

no

i did that intentionally so that its always a clean slate

mmm, right.

see that it's up to 7 minutes already, though. I don't really want to stick around waiting this long for it to build every time

especially if we need to do quick bugfix

yeah, that's a tradeoff true and if we build locally we do use the cache anyways i think.

the cache action actually caches docker images, rather than anything on the filesystem

so I think it's OK

one thing I don't know is how much space this is going to end up taking, and how much of our storage we're using

_lucifer: I finished https://github.com/metabrainz/brainzutils-pytho...

two more cache items to review before we release

maybe ruaok can take a look and see if we can find out how much we are using

but if it tops 5g for the repo it'll just evict old cache items.

yeah, that's what I hope

alastairp, nice :D. i'll check and update those PRs

alastairp: should i merge https://github.com/metabrainz/listenbrainz-serv...

_lucifer: our usage report for the last 30 days

thanks! :D

https://github.com/metabrainz/brainzutils-pytho... this is the output of the junit action, alastairp

very strange, the usage report has nothing on BU or LB.

I wonder if there's a different report

this seems to be repo storage, not actions storage

(maybe?)

the report I shared cam from "get usage report"

thanks. "github actions" shows 0 because it seems that it only tracks actions on private repos (public repos are free). Based on their docs I'd expect "Storage for actions and packages" to show our cache usage

but if that shows 0 then that's fine

_lucifer: I like that output!

I see that brainzutils cache increment does not have the ability to increment by more than 1.

I need to do that, however. should I just make multiple calls or set a new value?

in theory the latter can cause a race condition, but we only have one piece of code that inserts new listens right now, so it should never happen.

thoughts, alastairp, _lucifer ?

mm

ruaok: which method?

increment

I think that's a bug

we should expose INCRBY

that would be ideal

for now, can you either loop through, or manually call cache._r.incrby?

and we'll have a version later in the week

I'll do the latter.

[brainzutils-python] alastair opened pull request #66 (master…incr-amount): Allow increment to take an optional amount value https://github.com/metabrainz/brainzutils-pytho...

looks like .incr supports it too: https://github.com/andymccurdy/redis-py/blob/ma...

ruaok: we're probably at least another day away from releasing BU 2.0, so if you need it now, _r.incr is still the best way to go for now

k

alastairp: nice, so should we merge that junit action PR or keep it open for now?

_lucifer: let's merge it

there are some other available as well https://github.com/marketplace?type=actions&... if we want to take a look

😓

regarding having this in LB, we can reduce 4 comments to 1 by running all tests as different jobs in the same workflow

I don't really want to look into it more. do a github search and see which one is used most?

also, check for commit activity maybe?

oh yeah, that's a good idea for a future step

👍

_lucifer: looking through that list, many of them are false positives, I think there are only 2 or 3 which actually do what we need

yeah, the one we are already using is by far the most used.

cool. that's good enough for me

the latest docker says 'Docker Compose is now in the Docker CLI, try `docker compose up`'

finally, no need to install that as separate step.

neat

gonna be hell on my muscle memory