that's useful but i'd prefer it to be a python script maybe then?
2021-05-12 13217, 2021
ruaok
+10 to the last two suggestions.
2021-05-12 13221, 2021
alastairp
although the work that we've been doing the last few days allows us to git tag 'v-<current date>'; git push --tags; publish release on github; -> image is automatically built by github and pushed
2021-05-12 13203, 2021
alastairp
and so maybe we should be automating the creation of tags and publishing of releases instead?
2021-05-12 13208, 2021
_lucifer
we can also release just from the Github UI and set the tag there. it'll do the rest
2021-05-12 13227, 2021
alastairp
oh yeah, if it's a tag that doesn't exist it'll make it for you. I always do it the other way around
2021-05-12 13227, 2021
ruaok
alastairp: sounds like it, yes.
2021-05-12 13249, 2021
alastairp
BrainzBot: publish LB please
2021-05-12 13251, 2021
alastairp
^anyone?
2021-05-12 13200, 2021
ruaok
`docker/push.sh -t`makes and pushes a tag?
2021-05-12 13203, 2021
_lucifer
lol, noice
2021-05-12 13213, 2021
ruaok
ohh yeah.
2021-05-12 13217, 2021
_lucifer
no that tags the image only
2021-05-12 13239, 2021
ruaok
`docker/push.sh -t -p`?
2021-05-12 13245, 2021
alastairp
ruaok: we just had a discussion earlier this morning about how to push beta (and mayhem-test) images with push.sh
2021-05-12 13203, 2021
alastairp
so I'm getting a feeling that there are still a few use-cases that we need to support
2021-05-12 13224, 2021
ruaok
agreed.
2021-05-12 13234, 2021
alastairp
but yes. one entrypoint that lets us do all of these different things (with integration with github) sounds reasonable
2021-05-12 13239, 2021
alastairp
will open a ticket
2021-05-12 13252, 2021
alastairp
this takes us one step closer to our previous discussion about automating deploys more
2021-05-12 13253, 2021
ruaok
I for one would love push.sh to be more ruaok proof.
2021-05-12 13253, 2021
_lucifer
this part should take care of putting stuff in docker hub
2021-05-12 13237, 2021
_lucifer
the second part would be to take image from docker hub and update containers
2021-05-12 13245, 2021
alastairp
_lucifer: yeah
2021-05-12 13232, 2021
alastairp
ruaok: Mr_Monkey: _lucifer: how do you authenticate to github with the `git` command? ssh keys, or http + user + password/token
2021-05-12 13246, 2021
_lucifer
i use ssh.
2021-05-12 13251, 2021
Mr_Monkey
SSH too
2021-05-12 13201, 2021
ruaok
SSH
2021-05-12 13223, 2021
Mr_Monkey
Don't you shush me !
2021-05-12 13237, 2021
alastairp
_lucifer: I'm trying to see if there's a way to make authenticated API calls to github with ssh credentials, I can't find anything
2021-05-12 13259, 2021
alastairp
because if we wanted a script to automatically trigger a release it'd need to be authenticated somehow
2021-05-12 13220, 2021
_lucifer
i don't think there is. would need a token for that. even github cli needs that iirc.
[listenbrainz-server] mayhem opened pull request #1453 (master…listencount-expire-post-import): If a user does a bulk-import set a 4 hour expiry time on their listen… https://github.com/metabrainz/listenbrainz-server…
2021-05-12 13239, 2021
_lucifer
ruaok: regarding 1453, what if the user does a bulk import using say a python script?
2021-05-12 13253, 2021
ruaok
screw them.
2021-05-12 13206, 2021
_lucifer
lol
2021-05-12 13217, 2021
ruaok
gah. well spotted.
2021-05-12 13256, 2021
ruaok
I think we need a "reset my listen count" button.
2021-05-12 13259, 2021
_lucifer
ruaok, which exception to catch if I want to catch all psycopg2 specific errors, OperationalError or DatabaseError?
2021-05-12 13241, 2021
ruaok
I usually catch the former.
2021-05-12 13219, 2021
ruaok
but might as well catch, both, methinks.
2021-05-12 13253, 2021
_lucifer
👍
2021-05-12 13229, 2021
alastairp
did we start a document for "coding style and procedures"? this'd be a good thing to add
2021-05-12 13220, 2021
_lucifer
not yet, but its on my todo list :)
2021-05-12 13242, 2021
_lucifer
was thinking to do that when we take up code formatting.
2021-05-12 13209, 2021
_lucifer
alastairp, regarding exceptions this is what i currently think, create a `ExternalServiceError` have the invalid grant error and api error extend from it. i checked and there's almost ~20-25 places that these exceptions touch and catch one or the other.
2021-05-12 13237, 2021
alastairp
_lucifer: there is a metabrainz-guidelines repo I think - we should be able to reuse that
2021-05-12 13239, 2021
_lucifer
further, chave a ExternalServiceImporterErorr and use that in the spotify updater code.
2021-05-12 13246, 2021
_lucifer
*create
2021-05-12 13210, 2021
alastairp
that sounds fine
2021-05-12 13239, 2021
_lucifer
alastairp, yes, i remember that. intent is to add stuff to it and link it in the issue template.
2021-05-12 13206, 2021
_lucifer
*PR template
2021-05-12 13209, 2021
alastairp
cool
2021-05-12 13232, 2021
_lucifer
should the InvalidGrantError extend from APIErorr or the base error?
2021-05-12 13210, 2021
_lucifer
(currently i am not removing the api error just making it extend from the base external error to avoid breaking stuff)
2021-05-12 13215, 2021
alastairp
can you paste an overview of the exceptions + brief doc on each one?
I am putting that on hold. that is an improvement and I am still dealing with bugs.
2021-05-12 13218, 2021
ruaok
I'll get to it. I'm onto the next tricky mess.
2021-05-12 13221, 2021
_lucifer
👍 makes sense.
2021-05-12 13248, 2021
_lucifer
ruaok: 1453 approved
2021-05-12 13204, 2021
_lucifer
alastairp: can you look at 1452?
2021-05-12 13231, 2021
BrainzGit
[listenbrainz-server] mayhem merged pull request #1453 (master…listencount-expire-post-import): If a user does a bulk-import set a 4 hour expiry time on their listen… https://github.com/metabrainz/listenbrainz-server…
2021-05-12 13239, 2021
ruaok
_lucifer: thanks, please proceed.
2021-05-12 13225, 2021
_lucifer
yes, waiting a bit. if i can get 1452 in and i'll release using that :D
2021-05-12 13243, 2021
ruaok rushes to finish the next PR so we can keep this silly chain going
2021-05-12 13254, 2021
_lucifer
lol
2021-05-12 13258, 2021
alastairp
I mean, I'm happy to just accept _all_ PRs
2021-05-12 13202, 2021
alastairp
and then we can see what breaks
2021-05-12 13219, 2021
ruaok
and you'll do the usual post morterm PR reviews? :)
[listenbrainz-server] MonkeyDo merged pull request #1432 (master…dependabot/npm_and_yarn/lodash-4.17.21): [Security] Bump lodash from 4.17.19 to 4.17.21 https://github.com/metabrainz/listenbrainz-server…
2021-05-12 13241, 2021
_lucifer
alastairp, i have a unpushed commit for that. other than that i had taken a look i do not think its possible to consolidate more. i think it would make sense to just group the cron entries in a couple of scripts for easier management.
2021-05-12 13215, 2021
_lucifer
the entire spark_manage file is not needed, jsut need the request consumer command. moved that to manage.py
it> a tiny bit of cultural feedback for you _lucifer: "Yeah, right!" is normally used in the context for "You're full of shit!" rather than "oh, I agree with you." :) :)
2021-05-12 13245, 2021
shivam-kapila
Oh no didnt know that
2021-05-12 13210, 2021
_lucifer
this is the crontab and the file that contains the stats command.
2021-05-12 13252, 2021
_lucifer
there are only 2 stats commands and we probably want to retain that granularity. its that we need to call it with different args multiple times which is the issue.
2021-05-12 13259, 2021
alastairp
_lucifer: hmm, interesting
2021-05-12 13211, 2021
_lucifer
as you can see in the stats-crontab at the end we call a script to perform all the recommendation related tasks.
2021-05-12 13231, 2021
_lucifer
we should probably create 2-3 such files for various types of stats. thoughts?
2021-05-12 13250, 2021
alastairp
are you suggesting that we could have a single command that runs each of these stats?
2021-05-12 13257, 2021
alastairp
and just 1 cron line?
2021-05-12 13224, 2021
_lucifer
yes
2021-05-12 13231, 2021
alastairp
that sounds like a great idea
2021-05-12 13206, 2021
_lucifer
all the requests will queue up so it doesn't matter if we send too many at the same time. they'll be executed sequentially
2021-05-12 13216, 2021
alastairp
oh, right, even better
2021-05-12 13248, 2021
alastairp
that being said, I wonder if there are some possibilities for optimisation here
2021-05-12 13213, 2021
_lucifer
right, i am not sure. what other improvements are possible?
2021-05-12 13222, 2021
alastairp
e.g. are we doing extra work computing week/month/year stats separately? surely we should do them in 1 job
2021-05-12 13227, 2021
alastairp
because we've already loaded the data
2021-05-12 13254, 2021
_lucifer
iliekcomputers and ishaanshah had worked on that, they possibly know better than me
2021-05-12 13211, 2021
_lucifer
but i have seen spark mark stages as skipped
2021-05-12 13213, 2021
iliekcomputers
we run multiple jobs
2021-05-12 13223, 2021
iliekcomputers
seperately :D
2021-05-12 13240, 2021
_lucifer
hi iliekcomputers :D !
2021-05-12 13248, 2021
iliekcomputers
hello!
2021-05-12 13206, 2021
shivam-kapila
Damn the latency was better than Mr_Monkey. Hi 👋
2021-05-12 13210, 2021
_lucifer
right, we are discussing if it makes sense to do it in a single job
2021-05-12 13245, 2021
iliekcomputers
so the data is partitioned by month
2021-05-12 13253, 2021
Mr_Monkey
A wild iliekcomputers appears ! (Hello !)
2021-05-12 13201, 2021
iliekcomputers
so for the week/month job we just need to load up one file and run the query
2021-05-12 13210, 2021
iliekcomputers
there's not too much of an overhead
2021-05-12 13228, 2021
_lucifer
i have seen spark mark stages as skipped in the Spark UI dashboard, so it probably optimises some of it automatically as well.
2021-05-12 13231, 2021
iliekcomputers
for the year / all time job we have to load a number of files
2021-05-12 13231, 2021
alastairp
ah, there are partitions, cool
2021-05-12 13239, 2021
iliekcomputers
Mr_Monkey: 👋🏽
2021-05-12 13250, 2021
alastairp
Mr_Monkey: ?
2021-05-12 13202, 2021
iliekcomputers
if you want to refactor this, i think we should partition the data by days :D
2021-05-12 13211, 2021
iliekcomputers
that would make the incremental dump import much better.
2021-05-12 13220, 2021
iliekcomputers
but overall, i don't think it's a huge priority
2021-05-12 13226, 2021
alastairp
OK, that sounds like additional work
2021-05-12 13236, 2021
alastairp
--type=entity --range=week --entity=releases
2021-05-12 13240, 2021
_lucifer
yup, i have seen that ticket. its on the list, haven't got to it yet.
2021-05-12 13253, 2021
alastairp
what does this do? does it re-compute all data broken down into weeks?
2021-05-12 13257, 2021
ruaok
we'll need to work on the spark data export once we have the MBID mapping working well. perhaps we can do it then.
