Who created this bookbrainz ticket? Private investigator?😂
2021-05-13 13329, 2021
prabal
CatQuest: my college was in assam. I don't speak Assamese, but i can ask someone to translate it
2021-05-13 13310, 2021
_lucifer
alastairp: turns out using `url_for` in a background task is a hot mess. to use url_for, a request context must be available in addition to application context. in our case, the former isn't available. there is one fix, set SERVER_NAME, since we already have a SERVER_ROOT_URL config, it would be trivial to modify it to set SERVER_NAME. but setting SERVER_NAME has a side effect on how cookies work on subdomains.
2021-05-13 13326, 2021
_lucifer
cookies on beta.lb, test.lb and lb prod might interfere with each other if we do not configure it properly. for the time being, i think its best we revert to manually generating the url in spotify reader.
2021-05-13 13305, 2021
_lucifer
(we only use cookie for login, so login/logout on beta can also lead to the same happening on prod)
yvanzo: thanks! can we also add the nicks of admins to the introduction section at jira homepage or maybe create a dedicated community post where users can report spam?
2021-05-13 13339, 2021
_lucifer
like aerozol had mentioned about the spam above but they did not know who or where to report it.
2021-05-13 13312, 2021
Mr_Monkey
moin!
2021-05-13 13353, 2021
ruaok
moin!
2021-05-13 13313, 2021
ruaok
_lucifer: I got a suspcion that things were up last night. any idea on how to fix it?
2021-05-13 13318, 2021
_lucifer
ruaok, have been trying to debug for some time no leads yet. it seems that the reader is unable to refresh expired tokens. why so no idea. i have reverted all containers prior to youtube PR release for now.
2021-05-13 13343, 2021
ruaok
and the spice is flowing then?
2021-05-13 13344, 2021
_lucifer
things may still be broken for users who modified sptoify settings between the initial release and now
2021-05-13 13347, 2021
ruaok
eerrr, listens?
2021-05-13 13350, 2021
_lucifer
yes
2021-05-13 13318, 2021
_lucifer
listens are being imported for ~1800 users currently
_lucifer: just added a line to the introduction text: spam can be reported under OTHER project.
2021-05-13 13335, 2021
_lucifer
thanks yvanzo!
2021-05-13 13326, 2021
_lucifer
ruaok, yeah the spotify reader was broken ~8 hr last night and early morning today. and it still might be for a small number of users. so probably that's wehy.
2021-05-13 13332, 2021
_lucifer
also, since the timescale fixes were releases with youtube PR. they are also not in prod currently.
2021-05-13 13355, 2021
ruaok
not a big deal.
2021-05-13 13310, 2021
ruaok
let me wake up a bit more and I'll jump in to help think
today when i check spotify reader logs it was only running for 25 users.
2021-05-13 13320, 2021
_lucifer
I reverted spotify reader and prod to pre-youtube PR versions and it started to work fine, importing listens again for ~1800 users.
2021-05-13 13330, 2021
alastairp
right, so looks like there are 2 things going on here? 1 is that it can't refresh tokens, and the other that when it tries to report, it can't generate the error messages?
2021-05-13 13352, 2021
_lucifer
right, the unable to refresh tokens is the main issue.
2021-05-13 13359, 2021
alastairp
so the things that have changed since the last release and this one is the switch to the new table, and the upgrade of spotipy?
2021-05-13 13322, 2021
_lucifer
currently we are using old tables
2021-05-13 13335, 2021
alastairp
sure - so the new tables don't work?
2021-05-13 13349, 2021
_lucifer
right, new tables are not working as expected
2021-05-13 13316, 2021
_lucifer
they work as long as the access token works but once it expires things begin to fall apart
2021-05-13 13340, 2021
ruaok reads
2021-05-13 13349, 2021
alastairp
so, the places where things may have gone wrong: copying to the new table; reading tokens from the new tables with the rewrites to the spotify importer; something unexpected in spotipy 2.17
2021-05-13 13309, 2021
_lucifer
yes.
2021-05-13 13317, 2021
alastairp
let me recreate the old environment locally and set up spotify
2021-05-13 13325, 2021
alastairp
then I'll try the upgrade
2021-05-13 13330, 2021
_lucifer
i have done that. unable to reproduce locally
2021-05-13 13349, 2021
alastairp
interesting, so only some tokens are failing to refresh?
2021-05-13 13323, 2021
_lucifer
yeah. the errors did not occur at once.
2021-05-13 13329, 2021
alastairp
could it be something to do with scopes?
2021-05-13 13329, 2021
ruaok
could we isolate the spotifpy update from our own tables update?
2021-05-13 13342, 2021
alastairp
ruaok: yeah, that's a good idea
2021-05-13 13302, 2021
alastairp
_lucifer: remind me - when we use a refresh token to get a new access token does the refresh token change? or just the access token?
2021-05-13 13302, 2021
ruaok
because I really doubt it is our own tables. we'd catch an exception or whatnot.
2021-05-13 13315, 2021
_lucifer
we do not use spotipy to refresh tokens, so i do not think that is the issue. but i am clueless currently so we can try
2021-05-13 13326, 2021
ruaok
spotipy is a lot more opaque.
2021-05-13 13344, 2021
ruaok
lets eliminate it for our own sanity.
2021-05-13 13308, 2021
_lucifer
the refresh token may or may not change. we check the response and update out tables if spotify sends a new one
2021-05-13 13324, 2021
alastairp
ok, cool
2021-05-13 13348, 2021
ruaok
and you checked that the data written to DB is what you'd expect?
if the token expired between the first and second check, the failure we see would happen.
2021-05-13 13320, 2021
alastairp
that pattern of passing in a function from a class instance add some weird complexity
2021-05-13 13326, 2021
_lucifer
true.
2021-05-13 13316, 2021
_lucifer
i'll simplify this stuff. do you think this hypothesis could be correct ?
2021-05-13 13333, 2021
CatQuest
prabal: oh cool! thank you!
2021-05-13 13321, 2021
alastairp
any thoughts on why it wasn't a problem before?
2021-05-13 13358, 2021
_lucifer
maybe the first check worked earlier and now it doesn;t
2021-05-13 13331, 2021
ruaok
voodoo!
2021-05-13 13346, 2021
_lucifer
or sheer coincidence of timing
2021-05-13 13301, 2021
_lucifer
`token_expires < now() as token_expired`
2021-05-13 13309, 2021
_lucifer
🤦
2021-05-13 13350, 2021
alastairp
interesting, but that goes all the way back to my original version
2021-05-13 13301, 2021
alastairp
so we weren't using that value?
2021-05-13 13302, 2021
_lucifer
oh wait, that's right actually.
2021-05-13 13344, 2021
alastairp
but domain.Spotify.token_expired() actually recalculates it
2021-05-13 13315, 2021
_lucifer
that's removed in the new version.
2021-05-13 13313, 2021
alastairp
OK, I now understand what you're saying _lucifer. if the token has already expired, we refresh it and then go ahead with the update data load. this seems to work fine
2021-05-13 13341, 2021
alastairp
but if it hasn't expired, but then by the time that we get to it it has, we fail to refresh it properly
2021-05-13 13356, 2021
_lucifer
yes right
2021-05-13 13337, 2021
alastairp
`user['token_expired']` is actually a bad way of doing it - because this is true as of the moment that we read the database, not necessarily at the point that we process the user
2021-05-13 13340, 2021
_lucifer
according to what i see, this process has been broken since ever.
2021-05-13 13302, 2021
_lucifer
right.
2021-05-13 13332, 2021
_lucifer
i think we should not try to refresh it earlier, try to load data and then refresh correctly
2021-05-13 13348, 2021
alastairp
I think we should do it the other way around. keep this check, but compare against now() and the actual expiry time. add in a 10 minute leeway so that we refresh it before it expires
2021-05-13 13326, 2021
alastairp
LB-891 - from what I saw here it takes 7 minutes to rotate through all users as well
that might also explain why it worked earlier but not now. earlier the gap was only a few seconds, now the gap could be of almost ~7 min
2021-05-13 13308, 2021
alastairp
yeah
2021-05-13 13347, 2021
_lucifer
if we want to pre fetch refresh token, i think we should remove the check to refresh it during data load
2021-05-13 13324, 2021
alastairp
I think those are 2 clear improvements we can make: be smarter about checking the expiry of a token before we process; be smarter about the refresh when there is an error and how we call this function
