i also ran the migration again to avoid inconsistencies
2021-05-13 13352, 2021
alastairp
_lucifer: looking at the build process it seems like it decided to not use the cache when adding requirements.txt, not sure why that is
2021-05-13 13313, 2021
alastairp
but that being said, we should just use `docker tag imagname:v-whatever imagename:latest`
2021-05-13 13323, 2021
alastairp
which just makes a new tag with the same build
2021-05-13 13331, 2021
alastairp
instead of running build twice
2021-05-13 13332, 2021
_lucifer
ah! nice i'll update the scripts
2021-05-13 13341, 2021
_lucifer
i also install buildx in actions
2021-05-13 13302, 2021
_lucifer
i'll update docker server configs and other containers after an hour or so?
2021-05-13 13359, 2021
_lucifer
alastairp: two more things: i have opened the PR for running tests based on paths. i had tested on my fork first, where it worked as expected. second, i have updated the spark reader setup and associated docs in case you want to try again.
2021-05-13 13322, 2021
alastairp
_lucifer: there are many tokens which are expiring ~nowish
2021-05-13 13330, 2021
alastairp
so hopefully these will continue to update as expected
2021-05-13 13350, 2021
_lucifer
ah cool, we'll know soon :)
2021-05-13 13314, 2021
alastairp
_lucifer: any reason why you used file patterns instead of directories for the test paths?
2021-05-13 13330, 2021
alastairp
e.g. *.tsx instead of lsitenbrainz/webserver/static
2021-05-13 13315, 2021
BrainzGit
[listenbrainz-server] MonkeyDo merged pull request #1458 (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-13 13328, 2021
_lucifer
alastairp: no particular reason for tsx files. but there are js files elsewhere, hence it was easier to use file pattern for those.
2021-05-13 13358, 2021
alastairp
I was just talking to Mr_Monkey yesterday about it seems strange that the frontend is hidden away in such a dark corner of the source tree
2021-05-13 13318, 2021
alastairp
maybe we could have a small file shuffle to put things in a place that makes it easier to filter them
2021-05-13 13332, 2021
_lucifer
sure that makes sense.
2021-05-13 13312, 2021
alastairp
_lucifer: I ran `select musicbrainz_id, token_expires, now()>token_expires as expired from external_service_oauth oa join listens_importer li on oa.id=li.external_service_oauth_id join "user" u on u.id=li.user_id where error_message is null order by token_expires;`
2021-05-13 13325, 2021
alastairp
to find all upcoming expiring spotify tokens
2021-05-13 13327, 2021
Mr_Monkey
Which js files are outside of listenbrainz/webserver/static, _lucifer ?
2021-05-13 13344, 2021
alastairp
then I did docker logs -f on the spotify reader, and searched for the top item in the list
2021-05-13 13305, 2021
_lucifer
Mr_Monkey: the babel.config.js jest config eslint files etc in repository root
2021-05-13 13316, 2021
Mr_Monkey
Ah indeed
2021-05-13 13327, 2021
alastairp
the spotify reader imported all 1400 users, then started again, and it successfully processed the user who was at the top of the list of expired tokens
2021-05-13 13343, 2021
alastairp
the time of processing the user was about 6-7 minutes past the expiry time on the token
2021-05-13 13345, 2021
alastairp
so I think our original hypothesis of it being related to the is_expired being out of date by the time we got to it plus the bad refresh inside the do_query plus possibly the fact that a full cycle is starting to take longer caused us to get into this situation
2021-05-13 13346, 2021
alastairp
good work!
2021-05-13 13306, 2021
alastairp
_lucifer: should we remove the limitation of accounts on the youtube app?
2021-05-13 13322, 2021
_lucifer
oh nice to know we were right.
2021-05-13 13320, 2021
_lucifer
we should. lets discuss that in ~10 mins.
2021-05-13 13322, 2021
ruaok
ohhh someday I wish to have THAT feeling. :)
2021-05-13 13341, 2021
BrainzGit
[listenbrainz-server] dependabot-preview[bot] closed pull request #1438 (master…dependabot/npm_and_yarn/hosted-git-info-2.8.9): [Security] Bump hosted-git-info from 2.8.8 to 2.8.9 https://github.com/metabrainz/listenbrainz-server…
2021-05-13 13335, 2021
_lucifer is back
2021-05-13 13325, 2021
_lucifer
alastairp, we should apply for verification. the process is a bit involved. we need to record a video of the workflow as well and submit that.
2021-05-13 13338, 2021
alastairp
_lucifer: before or after we remove the limitation?
2021-05-13 13318, 2021
_lucifer
goes hand in hand, when we apply to remove the limitation. the form has to filled.
2021-05-13 13327, 2021
alastairp
ohhhh, I see
2021-05-13 13353, 2021
_lucifer
so we remove the limitation, then record the video and submit it
2021-05-13 13309, 2021
alastairp
wtf, that's stupid. because then we have this part of our website which is functional (and has to be functional so that they can see it??!?) but then no one can use it?
2021-05-13 13317, 2021
_lucifer
right
2021-05-13 13336, 2021
_lucifer
its total BS.
2021-05-13 13347, 2021
Mr_Monkey
Well they can use it, but they'll get that warning, right?
2021-05-13 13351, 2021
_lucifer
also, the player has to be up on all domains we want to get verified
2021-05-13 13317, 2021
_lucifer
we cannot get test verified without having the player running on it
2021-05-13 13307, 2021
_lucifer
so we some options here, remove test and beta from the google oauth dashboard and just get prod verified first
2021-05-13 13311, 2021
_lucifer
*we have
2021-05-13 13350, 2021
_lucifer
but in the meantime the player will stop working for all, test users including us
2021-05-13 13301, 2021
_lucifer
on those domains (beta and test)
2021-05-13 13302, 2021
alastairp
if we can get both prod and beta verified then that would be good
2021-05-13 13307, 2021
Mr_Monkey
my vote goes for deploying it on all servers and doing it in one go
2021-05-13 13340, 2021
_lucifer
we'll have to ensure that all servers are available at most times. as they might want to manually verify it.
2021-05-13 13308, 2021
alastairp
_lucifer: is there a document that explains all of these steps?
2021-05-13 13312, 2021
_lucifer
plus, all PRs deployed there should have the latest master merged in them so that they contain the youtube player
2021-05-13 13319, 2021
Mr_Monkey
Yep
2021-05-13 13322, 2021
_lucifer
yes, there are a few. let me get the link
2021-05-13 13324, 2021
alastairp
(one that we made, not a generic google help page)
2021-05-13 13337, 2021
_lucifer
no we don't have that
2021-05-13 13339, 2021
alastairp
I want a clear list of steps that we have to make, because I'm confused
2021-05-13 13355, 2021
sumedh has quit
2021-05-13 13305, 2021
alastairp
could you quickly write one up? I expect it's probably only 5-6 lines
_lucifer: at which stage do we remove you and me and Mr_Monkey and ruaok from the list of allowed users?
2021-05-13 13331, 2021
_lucifer
alastairp: we do not need to remove us.
2021-05-13 13359, 2021
alastairp
right now, if a user goes to the connect services page, can they log in to youtube?
2021-05-13 13326, 2021
_lucifer
no. not until we start filing the form.
2021-05-13 13325, 2021
_lucifer
the document describes the steps from once we start the process.
2021-05-13 13341, 2021
alastairp
> If you change any of the details that appear on your OAuth consent screen, such as the project's icon, display name, homepage or privacy policy URL, or authorized domains, you need to have your app re-verified for branding
2021-05-13 13352, 2021
alastairp
so we should definitely make sure that's correct before we start?
2021-05-13 13322, 2021
_lucifer
yes right. ruaok has access to the dashboard and i think he verified that the last time we attempted
_lucifer: we were just talking to ruaok. it's too late today to start this thing in progress, so we'll pick it up tomorrow
2021-05-13 13338, 2021
_lucifer
sure, makes sense.
2021-05-13 13340, 2021
alastairp
we just had a quick look at the app console
2021-05-13 13314, 2021
_lucifer
nice, i created an app on personal account to do the same.
2021-05-13 13315, 2021
alastairp
from what we understand, we can publish the app - taking it from "testing" to "production", and from that point in time, it'll be available to anyone but will show as being unverified
2021-05-13 13331, 2021
alastairp
and then after the verification process is complete, it'll show as verified
2021-05-13 13345, 2021
_lucifer
yes right, i see the same
2021-05-13 13355, 2021
_lucifer
so let's move to production?
2021-05-13 13303, 2021
alastairp
well, we can't now becaue ruaok has just gone home
2021-05-13 13315, 2021
_lucifer
ah ok, tomorrow then
2021-05-13 13326, 2021
alastairp
but yes, this is what I was trying to ask before - is there a way that we can allow other users to use the login, even if it's unverified
2021-05-13 13333, 2021
alastairp
but no problem, let's just deal with it all tomorrow
2021-05-13 13301, 2021
ruaok
I could when I get home in 5.
2021-05-13 13309, 2021
_lucifer
right, the docs don't mention it clearly. only when i put my test app in production i saw what happens
2021-05-13 13324, 2021
ruaok
But only for 2-3 mins
2021-05-13 13358, 2021
alastairp
one thing we weren't sure about right in the moment is at what point we need to "freeze" our name/icon/description
2021-05-13 13307, 2021
alastairp
before publishing to production, or before verification
2021-05-13 13310, 2021
alastairp
which is why we put it off
2021-05-13 13321, 2021
alastairp
no problem, tomorrow should be fine
2021-05-13 13338, 2021
alastairp
as ruaok said, if someone tweets at us, then that's great that we know someone is using the website and found the new feature!
2021-05-13 13322, 2021
_lucifer
sure. one other thing, you could test the google auth revoke now. as i ran the migration, the youtube tokens got deleted again without getting revoked.
2021-05-13 13352, 2021
_lucifer
so when you try to reconnect, i expect it to error with a missing refresh token.
2021-05-13 13301, 2021
_lucifer
that is if you had connected to youtube yesterday. Mr_Monkey and ruaok did so they'll have to revoke manually from myaccount google before connecting again.
2021-05-13 13345, 2021
alastairp
let me try, I haven't done anything since wednesday
that's an easy addition - we can make it retry 2-3 times if it fails
2021-05-13 13338, 2021
_lucifer
oh! nice.
2021-05-13 13355, 2021
_lucifer
i wonder why we hand wrote that logic for spotify 🤦
2021-05-13 13302, 2021
alastairp
haha
2021-05-13 13320, 2021
alastairp
yeah, it's not "simple" in requests - that is, there's not just a parameter that you put in the .get()
2021-05-13 13324, 2021
alastairp
but it's quite flexible
2021-05-13 13346, 2021
_lucifer
yup. makes sense.
2021-05-13 13358, 2021
alastairp
I have a session object that I use for scraping websites that sleeps for some random duration between queries in order to get around anti-scraping restrictions ;)
2021-05-13 13351, 2021
_lucifer
lol XD, nice work
2021-05-13 13358, 2021
_lucifer
all lb containers and docker service container updated.
2021-05-13 13333, 2021
_lucifer
ruaok, restarted api compat nginx as well. as you mentioned to do occasionally during upgrades :)
