one good thing to do here would be to do a check before we add the record - check if it doesn't exist otherwise add it (inside a transaction). or, do a INSERT ... ON CONFLICT DO UPDATE or similar
2021-05-18 13842, 2021
alastairp
nice work by mhor these days!
2021-05-18 13850, 2021
_lucifer
alastairp, the error in sentry and the last updated are ~7 min apart
2021-05-18 13802, 2021
imdeni joined the channel
2021-05-18 13822, 2021
_lucifer
ON CONFLICT DO UPDATE seems nice, we might even be able to simplify some of our disconect logic with it. what are your thoughts on it? we currently delete and then redirect to the external service for auth.
2021-05-18 13849, 2021
_lucifer
what if we just overwrote at time of insert and didn't bother disconnecting before.
2021-05-18 13811, 2021
hugo___ has quit
2021-05-18 13833, 2021
_lucifer
indeed nice work by mhor :D
2021-05-18 13855, 2021
alastairp
what's the behaviour of that page?
2021-05-18 13803, 2021
alastairp
mmm
2021-05-18 13818, 2021
alastairp
we redirect to spotify, then spotify redirects back to us (with a GET)
2021-05-18 13829, 2021
alastairp
I'm trying to work out what could have happened
2021-05-18 13821, 2021
hugo___ joined the channel
2021-05-18 13824, 2021
outsidecontext joined the channel
2021-05-18 13826, 2021
alastairp
if they press back (end up on spotify), then approve again, what happens? maybe that was the issue
2021-05-18 13857, 2021
alastairp
I'd have to think more about your suggestion for disconnecting - it doesn't feel like a good solution on the face of it, let me think it over more
2021-05-18 13858, 2021
_lucifer
Currently, when any button is clicked it sends a request to LB, LB disconnects if the user exists and then if the user has clicked any button other than disconnect, it redirects to the desired service with appropriate parameters.
2021-05-18 13855, 2021
_lucifer
My question was basically, is it a good idea to depend on ON CONFLICT DO UPDATE or if we have a chance to remove conflicts earlier manually we should do that?
2021-05-18 13856, 2021
_lucifer
if the user presses back anywhere on spotify's side, it should end up with and invalid grant error when we try to obtain the access code. that's what i have seen happening usually.
alastairp, should we try to get this one (there are a lot of moving parts here so probably best to wait until both you and ruaok have reviewed and tested) or do a release with the crontab fix to get dumps working again?
2021-05-18 13813, 2021
_lucifer
nice conditional action run already working, no spark or js tests triggered :)
2021-05-18 13821, 2021
sumedh joined the channel
2021-05-18 13835, 2021
SothoTalKer_ joined the channel
2021-05-18 13830, 2021
SothoTalKer has quit
2021-05-18 13832, 2021
sumedh has quit
2021-05-18 13810, 2021
paul_ joined the channel
2021-05-18 13852, 2021
fnurl has quit
2021-05-18 13843, 2021
paul_ has quit
2021-05-18 13816, 2021
fnurl joined the channel
2021-05-18 13859, 2021
fnurl has quit
2021-05-18 13814, 2021
fnurl joined the channel
2021-05-18 13825, 2021
alastairp
thanks for the confirmation about the invalid grant error. you're right, that's what I expected to see. so now I'm a bit confused about how this could have happened. let's slowly try some ideas to see if we can work out what happened. I suspect it'll be an easy fix
2021-05-18 13854, 2021
alastairp
_lucifer: how about we release now? with new BU and with the fixed crontab
2021-05-18 13859, 2021
alastairp
then we can release yours tomorrow
2021-05-18 13801, 2021
_lucifer
alastairp: yes that's what i was asking, release now or try to get that PR in.
2021-05-18 13812, 2021
_lucifer
let's do the release now.
2021-05-18 13825, 2021
_lucifer
i'll be available in ~1 hr, let's do it then?
2021-05-18 13856, 2021
alastairp
fien
2021-05-18 13858, 2021
alastairp
fine
2021-05-18 13836, 2021
sumedh joined the channel
2021-05-18 13834, 2021
akashgp09 joined the channel
2021-05-18 13820, 2021
alastairp
_lucifer: got no internet for some reason :(
2021-05-18 13802, 2021
_lucifer
oh!
2021-05-18 13816, 2021
_lucifer
i am available now. should i proceed with releasing or wait?
Consul Template returned errors: child process died with exit code -1
2021-05-18 13810, 2021
_lucifer
is it always like this?
2021-05-18 13832, 2021
alastairp
that's because the thing that you loaded in the exec{} block quit
2021-05-18 13842, 2021
_lucifer
right, so the python script didn't log any error when it crashed that's why its missing here?
2021-05-18 13855, 2021
alastairp
yes, likely
2021-05-18 13841, 2021
_lucifer
ah ok. that's strange but makes sense now. thanks!
2021-05-18 13807, 2021
_lucifer
another thing, i see the docker-node image is using a newer consul version than docker-python and iirc newer consul versions have some things that tie in startup improvements PR, should we consider upgrading as well?
2021-05-18 13844, 2021
alastairp
no, new consul version shouldn't have anything to do with startup improvements
2021-05-18 13850, 2021
yvanzo has quit
2021-05-18 13811, 2021
alastairp
we have this ongoing process to slowly upgrade consul, consul template, and base OS versions
2021-05-18 13816, 2021
alastairp
but that's going slowly
2021-05-18 13845, 2021
_lucifer
makes sense.
2021-05-18 13813, 2021
_lucifer
but Ubuntu 16.04 recently reached EOL we should probably priortise on upgrading it methinks
2021-05-18 13816, 2021
alastairp
yes, it's been a pending task for a while now
2021-05-18 13821, 2021
alastairp
like everything else
2021-05-18 13802, 2021
_lucifer
yeah. what would be the process like to go about the ubuntu upgrade?
2021-05-18 13827, 2021
alastairp
we're in the middle of replacing some tools that integrate with consul, because they're out of date
2021-05-18 13840, 2021
alastairp
zas started work on making some new replacements, but they need to be tested
when i got rid of the docker container, i forgot that the machines are on a different ubuntu version.
2021-05-18 13809, 2021
_lucifer
20.04 has removed the pxz package so the request consumer had errored.
2021-05-18 13843, 2021
alastairp
this is one nice thing about having docker in place - we know that building the dockerfile will install of the required dependencies
2021-05-18 13806, 2021
alastairp
we could look at getting some automation tools in place to install all of the dependencies for the spark cluster
2021-05-18 13842, 2021
_lucifer
sure, but it actually shouldn't need anything extra. i was unaware of `pxz` till yesterday so i'll check the code base once in detail to make sure nothing else is missing.
2021-05-18 13800, 2021
alastairp
until we reinstall the cluster and forget it
2021-05-18 13832, 2021
_lucifer
we don't need pxz anymore, i have replaced it with xz as that has superseded pxz.
2021-05-18 13853, 2021
_lucifer
but right, i'll add the xz requirement to syswiki
2021-05-18 13841, 2021
_lucifer
i remembered you had mentioned ansible once, not sure how that works though
2021-05-18 13804, 2021
_lucifer
documented `xz` requirement in syswiki.
2021-05-18 13846, 2021
alastairp
great. let's talk about this at a later time, then
2021-05-18 13850, 2021
yvanzo joined the channel
2021-05-18 13813, 2021
_lucifer
sure, thanks for the discussion!
2021-05-18 13834, 2021
alastairp
_lucifer: interesting, I tried to reproduce the spotify issue by loading up the connect page in 2 tabs, doing 1 and then doing the same on the other
2021-05-18 13840, 2021
alastairp
and I didn't get the error
2021-05-18 13853, 2021
alastairp
so yeah, some weird situation going on here
2021-05-18 13839, 2021
_lucifer
yeah, you shouldn't get an error because we always try to disconnect before connecting.
2021-05-18 13805, 2021
_lucifer
more so there was a 7 min gap in the error's when happened and the user's last updated time.
2021-05-18 13810, 2021
alastairp
ah, I see - now I understand what you mean when you said that we disconnect
yup, i specifically left a comment there otherwise, 2 months later when fixing something else i would probably go on to delete the check here forgetting about this use case.
