@fettuccinae:matrix.org: I have some question on your pausing PR: Why are you adding pause/unpause to background tasks?
fettuccinae[m]
mayhem[m]: i was trying to implement pause and unpause function the way delete_user function is implemented
mayhem[m]
this doesn't make any sense to me. the background stuff us for a momentary task that needs to be executed. pausing a user is not that.
I dont see why that is needed? what does it solve?
fettuccinae[m]
if i didnt add it in background tasks, i'd had to set up an endpoint just for pausing users.
mayhem[m]
adding an endpoint sounds about right to me.
fettuccinae[m]
and i added the pause button in "with select" instead of edit options because edit options was just flipping flag without sending email
mayhem[m]: i will do it
and about timescale reader
writer*
is it necessary to send the flag in pipeline because importers are taking care of processing that user right?
mayhem[m]
ahhh, I see the pause/unpause under with selected. cool.
no need to send the flag in the pipeline. if the user is paused, the timescale writer should just drop the listen.
fettuccinae[m]
the data related to that user doesnt goes into timescale writer due to impoters dropping them already, shall i still add a conditional in timescale writer?
mayhem[m]
the importers are only one way that we can get data. (both spotify and last.fm -- we need to make sure that both are covered). but the user can submit listens via the /submit endpoint.
actually, we should add the check in the /submit endpoint not in the timescale writer, come to think of it.
fettuccinae[m]
mayhem[m]: i did check it in /submit endpoint
if user['is_paused']:
raise APIUnauthorized("user_id is paused and is currently not accepting listens. Feel free to Contact Us if you have any questions about this.")
mayhem[m]
yes, ok. great!
no need to do anything else on that front then.
I would say, just undo the background stuff, make an API endpoint and I think that should be it.
fettuccinae[m]
mayhem[m]: alright
MyNetAz has quit
d4rkie has quit
d4rkie joined the channel
reosarevok[m]
Unrelatedly, a bunch of files seem to be missing ending newlines
Is there a reason we would even send an automatic email? Is it for cases where you're expecting the person pausing it won't actually be the one sending the actual "what you did" email?
There's no need for "Contact Us" to be uppercase
mayhem[m]
reosarevok[m]: context?
reosarevok[m]: context?\
reosarevok[m]
In that PR
In general
mayhem[m]
reosarevok[m]: I guess not.
reosarevok[m]
And also in the PR in general, all the communication / error messages seem to uppercase "Contact Us" in the text for no clear reason
mayhem[m]
reosarevok[m]: leave a comment on the PR?
reosarevok[m]
I can, I just was asking here in case it's not LB policy to have the end newlines
mayhem[m]
reosarevok[m]: I think that is a good idea in general. in case you're slacking.
reosarevok[m]
Since I know you're usually pretty whatever about code style :)
mayhem[m]
which in your case....
reosarevok[m]
But sure, I'll just leave notes
mayhem[m]
reosarevok[m]: I am??
reosarevok[m]
Well, I remember many cases of you complaining about style warnings and whatnot :) But maybe it's different nowadays
Anyway, on it
Hmm, I was going to complain about "contact us {{ url }}" but does the email system even support URLs with titles?
(ideally the link would be "contact us" itself, but I don't find any examples of you already doing that so I guess maybe it's not supported)
mayhem[m]
not supported. we're sending plain text emails.
I'm hoping that eventually we can use Jade (try @jade:ellis.link) 's templates for all these.
that comment applies to the notificiations as well.
